chiark / gitweb /
logind: save/restore session devices and their respective file descriptors
authorFranck Bui <fbui@suse.com>
Thu, 16 Mar 2017 10:17:09 +0000 (11:17 +0100)
committerSven Eden <yamakuzure@gmx.net>
Tue, 25 Jul 2017 07:46:51 +0000 (09:46 +0200)
This patch ensures that session devices are saved for each session.

In order to make the revokation logic work when logind is restarted, the
session devices are now saved in the session state files and their respective
file descriptors sent to PID1's fdstore in order to keep them open accross
restart.

This is mandatory in order to keep the revokation logic working. Indeed in case
of input-devices, the same file descriptors must be shared by logind and a
given session controller in order EVIOCREVOKE to work otherwise multiple
sessions can have device access in parallel.

This should be the only remaining and missing piece for making logind fully
restartable.

Fixes: #1163

src/login/logind-session-dbus.c
src/login/logind-session-device.c
src/login/logind-session-device.h
src/login/logind-session.c
src/login/logind-session.h
src/login/logind.c

index a394865..1316c50 100644 (file)
@@ -396,7 +396,7 @@ static int method_take_control(sd_bus_message *message, void *userdata, sd_bus_e
         if (uid != 0 && (force || uid != s->user->uid))
                 return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Only owner of session may take control");
 
-        r = session_set_controller(s, sd_bus_message_get_sender(message), force);
+        r = session_set_controller(s, sd_bus_message_get_sender(message), force, true);
         if (r < 0)
                 return r;
 
@@ -444,14 +444,23 @@ static int method_take_device(sd_bus_message *message, void *userdata, sd_bus_er
                  * equivalent). */
                 return sd_bus_error_setf(error, BUS_ERROR_DEVICE_IS_TAKEN, "Device already taken");
 
-        r = session_device_new(s, dev, &sd);
+        r = session_device_new(s, dev, true, &sd);
         if (r < 0)
                 return r;
 
+        r = session_device_save(sd);
+        if (r < 0)
+                goto error;
+
         r = sd_bus_reply_method_return(message, "hb", sd->fd, !sd->active);
         if (r < 0)
-                session_device_free(sd);
+                goto error;
+
+        session_save(s);
+        return 0;
 
+error:
+        session_device_free(sd);
         return r;
 }
 
@@ -478,6 +487,8 @@ static int method_release_device(sd_bus_message *message, void *userdata, sd_bus
                 return sd_bus_error_setf(error, BUS_ERROR_DEVICE_NOT_TAKEN, "Device not taken");
 
         session_device_free(sd);
+        session_save(s);
+
         return sd_bus_reply_method_return(message, NULL);
 }
 
index 6c65607..915f87d 100644 (file)
@@ -210,7 +210,10 @@ static int session_device_start(SessionDevice *sd) {
                 r = session_device_open(sd, true);
                 if (r < 0)
                         return r;
-                close_nointr(sd->fd);
+                /* For evdev devices, the file descriptor might be left
+                 * uninitialized. This might happen while resuming into a
+                 * session and logind has been restarted right before. */
+                safe_close(sd->fd);
                 sd->fd = r;
                 break;
         case DEVICE_TYPE_UNKNOWN:
@@ -345,7 +348,7 @@ err_dev:
         return r;
 }
 
-int session_device_new(Session *s, dev_t dev, SessionDevice **out) {
+int session_device_new(Session *s, dev_t dev, bool open_device, SessionDevice **out) {
         SessionDevice *sd;
         int r;
 
@@ -374,22 +377,24 @@ int session_device_new(Session *s, dev_t dev, SessionDevice **out) {
                 goto error;
         }
 
-        /* Open the device for the first time. We need a valid fd to pass back
-         * to the caller. If the session is not active, this _might_ immediately
-         * revoke access and thus invalidate the fd. But this is still needed
-         * to pass a valid fd back. */
-        sd->active = session_is_active(s);
-        r = session_device_open(sd, sd->active);
-        if (r < 0) {
-                /* EINVAL _may_ mean a master is active; retry inactive */
-                if (sd->active && r == -EINVAL) {
-                        sd->active = false;
-                        r = session_device_open(sd, false);
+        if (open_device) {
+                /* Open the device for the first time. We need a valid fd to pass back
+                 * to the caller. If the session is not active, this _might_ immediately
+                 * revoke access and thus invalidate the fd. But this is still needed
+                 * to pass a valid fd back. */
+                sd->active = session_is_active(s);
+                r = session_device_open(sd, sd->active);
+                if (r < 0) {
+                        /* EINVAL _may_ mean a master is active; retry inactive */
+                        if (sd->active && r == -EINVAL) {
+                                sd->active = false;
+                                r = session_device_open(sd, false);
+                        }
+                        if (r < 0)
+                                goto error;
                 }
-                if (r < 0)
-                        goto error;
+                sd->fd = r;
         }
-        sd->fd = r;
 
         LIST_PREPEND(sd_by_device, sd->device->session_devices, sd);
 
@@ -439,15 +444,16 @@ void session_device_complete_pause(SessionDevice *sd) {
 void session_device_resume_all(Session *s) {
         SessionDevice *sd;
         Iterator i;
-        int r;
 
         assert(s);
 
         HASHMAP_FOREACH(sd, s->devices, i) {
                 if (!sd->active) {
-                        r = session_device_start(sd);
-                        if (!r)
-                                session_device_notify(sd, SESSION_DEVICE_RESUME);
+                        if (session_device_start(sd) < 0)
+                                continue;
+                        if (session_device_save(sd) < 0)
+                                continue;
+                        session_device_notify(sd, SESSION_DEVICE_RESUME);
                 }
         }
 }
@@ -482,3 +488,36 @@ unsigned int session_device_try_pause_all(Session *s) {
 
         return num_pending;
 }
+
+int session_device_save(SessionDevice *sd) {
+        _cleanup_free_ char *state = NULL;
+        int r;
+
+        assert(sd);
+
+        /* Store device fd in PID1. It will send it back to us on
+         * restart so revocation will continue to work. To make things
+         * simple, send fds for all type of devices even if they don't
+         * support the revocation mechanism so we don't have to handle
+         * them differently later.
+         *
+         * Note: for device supporting revocation, PID1 will drop a
+         * stored fd automatically if the corresponding device is
+         * revoked. */
+        r = asprintf(&state, "FDSTORE=1\n"
+                             "FDNAME=session-%s", sd->session->id);
+        if (r < 0)
+                return -ENOMEM;
+
+        return sd_pid_notify_with_fds(0, false, state, &sd->fd, 1);
+}
+
+void session_device_attach_fd(SessionDevice *sd, int fd, bool active) {
+        assert(fd > 0);
+        assert(sd);
+        assert(sd->fd < 0);
+        assert(!sd->active);
+
+        sd->fd = fd;
+        sd->active = active;
+}
index 7c85035..83aef1e 100644 (file)
@@ -44,10 +44,13 @@ struct SessionDevice {
         LIST_FIELDS(struct SessionDevice, sd_by_device);
 };
 
-int session_device_new(Session *s, dev_t dev, SessionDevice **out);
+int session_device_new(Session *s, dev_t dev, bool open_device, SessionDevice **out);
 void session_device_free(SessionDevice *sd);
 void session_device_complete_pause(SessionDevice *sd);
 
 void session_device_resume_all(Session *s);
 void session_device_pause_all(Session *s);
 unsigned int session_device_try_pause_all(Session *s);
+
+int session_device_save(SessionDevice *sd);
+void session_device_attach_fd(SessionDevice *sd, int fd, bool active);
index bd2aac9..a2709f3 100644 (file)
@@ -154,6 +154,18 @@ void session_set_user(Session *s, User *u) {
         LIST_PREPEND(sessions_by_user, u->sessions, s);
 }
 
+static void session_save_devices(Session *s, FILE *f) {
+        SessionDevice *sd;
+        Iterator i;
+
+        if (!hashmap_isempty(s->devices)) {
+                fprintf(f, "DEVICES=");
+                HASHMAP_FOREACH(sd, s->devices, i)
+                        fprintf(f, "%u:%u ", major(sd->dev), minor(sd->dev));
+                fprintf(f, "\n");
+        }
+}
+
 int session_save(Session *s) {
         _cleanup_free_ char *temp_path = NULL;
         _cleanup_fclose_ FILE *f = NULL;
@@ -285,8 +297,10 @@ int session_save(Session *s) {
                         s->timestamp.realtime,
                         s->timestamp.monotonic);
 
-        if (s->controller)
+        if (s->controller) {
                 fprintf(f, "CONTROLLER=%s\n", s->controller);
+                session_save_devices(s, f);
+        }
 
         r = fflush_and_check(f);
         if (r < 0)
@@ -308,6 +322,43 @@ fail:
         return log_error_errno(r, "Failed to save session data %s: %m", s->state_file);
 }
 
+static int session_load_devices(Session *s, const char *devices) {
+        const char *p;
+        int r = 0;
+
+        assert(s);
+
+        for (p = devices;;) {
+                _cleanup_free_ char *word = NULL;
+                SessionDevice *sd;
+                dev_t dev;
+                int k;
+
+                k = extract_first_word(&p, &word, NULL, 0);
+                if (k == 0)
+                        break;
+                if (k < 0) {
+                        r = k;
+                        break;
+                }
+
+                k = parse_dev(word, &dev);
+                if (k < 0) {
+                        r = k;
+                        continue;
+                }
+
+                /* The file descriptors for loaded devices will be reattached later. */
+                k = session_device_new(s, dev, false, &sd);
+                if (k < 0)
+                        r = k;
+        }
+
+        if (r < 0)
+                log_error_errno(r, "Loading session devices for session %s failed: %m", s->id);
+
+        return r;
+}
 
 int session_load(Session *s) {
         _cleanup_free_ char *remote = NULL,
@@ -321,7 +372,9 @@ int session_load(Session *s) {
                 *uid = NULL,
                 *realtime = NULL,
                 *monotonic = NULL,
-                *controller = NULL;
+                *controller = NULL,
+                *active = NULL,
+                *devices = NULL;
 
         int k, r;
 
@@ -351,6 +404,8 @@ int session_load(Session *s) {
                            "REALTIME",       &realtime,
                            "MONOTONIC",      &monotonic,
                            "CONTROLLER",     &controller,
+                           "ACTIVE",         &active,
+                           "DEVICES",        &devices,
                            NULL);
 
         if (r < 0)
@@ -453,10 +508,17 @@ int session_load(Session *s) {
         if (monotonic)
                 timestamp_deserialize(monotonic, &s->timestamp.monotonic);
 
+        if (active) {
+                k = parse_boolean(active);
+                if (k >= 0)
+                        s->was_active = k;
+        }
+
         if (controller) {
-                if (bus_name_has_owner(s->manager->bus, controller, NULL) > 0)
-                        session_set_controller(s, controller, false);
-                else
+                if (bus_name_has_owner(s->manager->bus, controller, NULL) > 0) {
+                        session_set_controller(s, controller, false, false);
+                        session_load_devices(s, devices);
+                } else
                         session_restore_vt(s);
         }
 
@@ -1254,7 +1316,7 @@ static int on_bus_track(sd_bus_track *track, void *userdata) {
         return 0;
 }
 
-int session_set_controller(Session *s, const char *sender, bool force) {
+int session_set_controller(Session *s, const char *sender, bool force, bool prepare) {
         _cleanup_free_ char *name = NULL;
         int r;
 
@@ -1286,11 +1348,14 @@ int session_set_controller(Session *s, const char *sender, bool force) {
          * Note that we reset the VT on ReleaseControl() and if the controller
          * exits.
          * If logind crashes/restarts, we restore the controller during restart
-         * or reset the VT in case it crashed/exited, too. */
-        r = session_prepare_vt(s);
-        if (r < 0) {
-                s->track = sd_bus_track_unref(s->track);
-                return r;
+         * (without preparing the VT since the controller has probably overridden
+         * VT state by now) or reset the VT in case it crashed/exited, too. */
+        if (prepare) {
+                r = session_prepare_vt(s);
+                if (r < 0) {
+                        s->track = sd_bus_track_unref(s->track);
+                        return r;
+                }
         }
 
         session_release_controller(s, true);
index d86ba61..1e7f3ad 100644 (file)
@@ -113,6 +113,8 @@ struct Session {
         bool started:1;
         bool stopping:1;
 
+        bool was_active:1;
+
         sd_bus_message *create_message;
 
         sd_event_source *timer_event_source;
@@ -178,7 +180,7 @@ void session_restore_vt(Session *s);
 void session_leave_vt(Session *s);
 
 bool session_is_controller(Session *s, const char *sender);
-int session_set_controller(Session *s, const char *sender, bool force);
+int session_set_controller(Session *s, const char *sender, bool force, bool prepare);
 void session_drop_controller(Session *s);
 
 int bus_session_method_activate(sd_bus_message *message, void *userdata, sd_bus_error *error);
index 83b06af..d46358e 100644 (file)
@@ -436,10 +436,71 @@ static int manager_enumerate_users(Manager *m) {
         return r;
 }
 
+static int manager_attach_fds(Manager *m) {
+        _cleanup_strv_free_ char **fdnames = NULL;
+        int n, i, fd;
+
+        /* Upon restart, PID1 will send us back all fds of session devices
+         * that we previously opened. Each file descriptor is associated
+         * with a given session. The session ids are passed through FDNAMES. */
+
+        n = sd_listen_fds_with_names(true, &fdnames);
+        if (n <= 0)
+                return n;
+
+        for (i = 0; i < n; i++) {
+                struct stat st;
+                SessionDevice *sd;
+                Session *s;
+                char *id;
+
+                fd = SD_LISTEN_FDS_START + i;
+
+                id = startswith(fdnames[i], "session-");
+                if (!id)
+                        continue;
+
+                s = hashmap_get(m->sessions, id);
+                if (!s) {
+                        /* If the session doesn't exist anymore, the associated session
+                         * device attached to this fd doesn't either. Let's simply close
+                         * this fd. */
+                        log_debug("Failed to attach fd for unknown session: %s", id);
+                        close_nointr(fd);
+                        continue;
+                }
+
+                if (fstat(fd, &st) < 0) {
+                        /* The device is allowed to go away at a random point, in which
+                         * case fstat failing is expected. */
+                        log_debug_errno(errno, "Failed to stat device fd for session %s: %m", id);
+                        close_nointr(fd);
+                        continue;
+                }
+
+                sd = hashmap_get(s->devices, &st.st_rdev);
+                if (!sd) {
+                        /* Weird we got an fd for a session device which wasn't
+                        * recorded in the session state file... */
+                        log_warning("Got fd for missing session device [%u:%u] in session %s",
+                                    major(st.st_rdev), minor(st.st_rdev), s->id);
+                        close_nointr(fd);
+                        continue;
+                }
+
+                log_debug("Attaching fd to session device [%u:%u] for session %s",
+                          major(st.st_rdev), minor(st.st_rdev), s->id);
+
+                session_device_attach_fd(sd, fd, s->was_active);
+        }
+
+        return 0;
+}
+
 static int manager_enumerate_sessions(Manager *m) {
         _cleanup_closedir_ DIR *d = NULL;
         struct dirent *de;
-        int r = 0;
+        int r = 0, k;
 
         assert(m);
 
@@ -454,7 +515,6 @@ static int manager_enumerate_sessions(Manager *m) {
 
         FOREACH_DIRENT(de, d, return -errno) {
                 struct Session *s;
-                int k;
 
                 if (!dirent_is_file(de))
                         continue;
@@ -468,7 +528,6 @@ static int manager_enumerate_sessions(Manager *m) {
                 k = manager_add_session(m, de->d_name, &s);
                 if (k < 0) {
                         log_error_errno(k, "Failed to add session by file name %s: %m", de->d_name);
-
                         r = k;
                         continue;
                 }
@@ -480,6 +539,12 @@ static int manager_enumerate_sessions(Manager *m) {
                         r = k;
         }
 
+        /* We might be restarted and PID1 could have sent us back the
+         * session device fds we previously saved. */
+        k = manager_attach_fds(m);
+        if (k < 0)
+                log_warning_errno(k, "Failed to reattach session device fds: %m");
+
         return r;
 }
 
@@ -1047,10 +1112,10 @@ static int manager_parse_config_file(Manager *m) {
         assert(m);
 
         return config_parse_many_nulstr(PKGSYSCONFDIR "/logind.conf",
-                                 CONF_PATHS_NULSTR("systemd/logind.conf.d"),
-                                 "Login\0",
-                                 config_item_perf_lookup, logind_gperf_lookup,
-                                 false, m);
+                                        CONF_PATHS_NULSTR("systemd/logind.conf.d"),
+                                        "Login\0",
+                                        config_item_perf_lookup, logind_gperf_lookup,
+                                        false, m);
 #else
          const char* logind_conf = getenv("ELOGIND_CONF_FILE");