From 6179ccf82559f78ee6526452d6052b4a964f7f3b Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 16 Mar 2017 11:17:09 +0100 Subject: [PATCH 1/1] logind: save/restore session devices and their respective file descriptors 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 | 17 ++++-- src/login/logind-session-device.c | 79 +++++++++++++++++++++------- src/login/logind-session-device.h | 5 +- src/login/logind-session.c | 87 +++++++++++++++++++++++++++---- src/login/logind-session.h | 4 +- src/login/logind.c | 79 +++++++++++++++++++++++++--- 6 files changed, 228 insertions(+), 43 deletions(-) diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c index a39486527..1316c5008 100644 --- a/src/login/logind-session-dbus.c +++ b/src/login/logind-session-dbus.c @@ -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); } diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index 6c65607b9..915f87dd5 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -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; +} diff --git a/src/login/logind-session-device.h b/src/login/logind-session-device.h index 7c8503583..83aef1e18 100644 --- a/src/login/logind-session-device.h +++ b/src/login/logind-session-device.h @@ -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); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index bd2aac99a..a2709f3d0 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -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); diff --git a/src/login/logind-session.h b/src/login/logind-session.h index d86ba61ad..1e7f3ad49 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -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); diff --git a/src/login/logind.c b/src/login/logind.c index 83b06afa3..d46358e31 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -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"); -- 2.30.2