logind: refuse overriding idle hint on tty sessions

Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it as idle again it we'd just revert to automatic TTY
atime idle detection, thus making it impossible to mark the session as
non-idle, unless its TTY is atime-touched all the time. But of course,
marking a session as idle is pretty much fatal if you never can mark it
as non-idle again.

This change is triggred by bug reports such as this:

https://github.com/systemd/systemd/issues/14053

With this patch we will now output a clean, clear error message if a
client tries to manipulate the idle state of a non-graphical session.
This means we now have clear rules: "manual" idle logic for graphical
sessions, and TTY based ones for all others that have a TTY of some
form.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

(Also includes some other fixes and clean-ups in related code)

Closes: #14053

diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 8f13edafb00fd991ba447b208fa4c412a17b698a..32a64fff0cf131b36c65ddc1671e429a31e6cda5 100644 (file)
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -249,7 +249,11 @@ static int method_set_idle_hint(sd_bus_message *message, void *userdata, sd_bus_
         if (uid != 0 && uid != s->user->uid)
                 return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Only owner of session may set idle hint");
 
-        session_set_idle_hint(s, b);
+        r = session_set_idle_hint(s, b);
+        if (r == -ENOTTY)
+                return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Idle hint control is not supported on non-graphical sessions.");
+        if (r < 0)
+                return r;
 
         return sd_bus_reply_method_return(message, NULL);
 }

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 1ef73570e58b52bf35d4ae968dd62e129dd736c2..a9be416f52d790308646c8071e10fb7f9f320548 100644 (file)
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -932,63 +932,57 @@ static int get_process_ctty_atime(pid_t pid, usec_t *atime) {
 }
 
 int session_get_idle_hint(Session *s, dual_timestamp *t) {
-        usec_t atime = 0, n;
+        usec_t atime = 0;
         int r;
 
         assert(s);
 
-        /* Explicit idle hint is set */
-        if (s->idle_hint) {
+        /* Graphical sessions have an explicit idle hint */
+        if (SESSION_TYPE_IS_GRAPHICAL(s->type)) {
                 if (t)
                         *t = s->idle_hint_timestamp;
 
                 return s->idle_hint;
         }
 
-        /* Graphical sessions should really implement a real
-         * idle hint logic */
-        if (SESSION_TYPE_IS_GRAPHICAL(s->type))
-                goto dont_know;
-
-        /* For sessions with an explicitly configured tty, let's check
-         * its atime */
+        /* For sessions with an explicitly configured tty, let's check its atime */
         if (s->tty) {
                 r = get_tty_atime(s->tty, &atime);
                 if (r >= 0)
                         goto found_atime;
         }
 
-        /* For sessions with a leader but no explicitly configured
-         * tty, let's check the controlling tty of the leader */
+        /* For sessions with a leader but no explicitly configured tty, let's check the controlling tty of
+         * the leader */
         if (pid_is_valid(s->leader)) {
                 r = get_process_ctty_atime(s->leader, &atime);
                 if (r >= 0)
                         goto found_atime;
         }
 
-dont_know:
         if (t)
-                *t = s->idle_hint_timestamp;
+                *t = DUAL_TIMESTAMP_NULL;
 
-        return 0;
+        return false;
 
 found_atime:
         if (t)
                 dual_timestamp_from_realtime(t, atime);
 
-        n = now(CLOCK_REALTIME);
-
         if (s->manager->idle_action_usec <= 0)
-                return 0;
+                return false;
 
-        return atime + s->manager->idle_action_usec <= n;
+        return usec_add(atime, s->manager->idle_action_usec) <= now(CLOCK_REALTIME);
 }
 
-void session_set_idle_hint(Session *s, bool b) {
+int session_set_idle_hint(Session *s, bool b) {
         assert(s);
 
+        if (!SESSION_TYPE_IS_GRAPHICAL(s->type))
+                return -ENOTTY;
+
         if (s->idle_hint == b)
-                return;
+                return 0;
 
         s->idle_hint = b;
         dual_timestamp_get(&s->idle_hint_timestamp);
@@ -1000,6 +994,8 @@ void session_set_idle_hint(Session *s, bool b) {
 
         user_send_changed(s->user, "IdleHint", "IdleSinceHint", "IdleSinceHintMonotonic", NULL);
         manager_send_changed(s->manager, "IdleHint", "IdleSinceHint", "IdleSinceHintMonotonic", NULL);
+
+        return 1;
 }
 
 int session_get_locked_hint(Session *s) {

diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 28b01d2b9a63a688ee5af0612ced813266ee1c6e..c51392bef655ef533c8d5ed95313aeabab69d269 100644 (file)
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -132,7 +132,7 @@ void session_add_to_gc_queue(Session *s);
 int session_activate(Session *s);
 bool session_is_active(Session *s);
 int session_get_idle_hint(Session *s, dual_timestamp *t);
-void session_set_idle_hint(Session *s, bool b);
+int session_set_idle_hint(Session *s, bool b);
 int session_get_locked_hint(Session *s);
 void session_set_locked_hint(Session *s, bool b);
 int session_create_fifo(Session *s);