From 2c0a91fedc150195f54d91bf75515e4bfff0d753 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 29 Sep 2015 11:36:18 +0200 Subject: [PATCH] login: fix re-use of users If the last reference to a user is released, we queue stop-jobs for the user-service and slice. Only once those are finished, we drop the user-object. However, if a new session is opened before the user object is fully dropped, we currently incorrectly re-use the object. This has the effect, that we get stale sessions without a valid "elogind --user" instance. Fix this by properly allowing user_start() to be called, even if user->stopping is true. --- src/login/logind-user.c | 43 ++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 0f7f31aa8..27f737000 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -481,15 +481,32 @@ int user_start(User *u) { assert(u); - if (u->started) + if (u->started && !u->stopping) return 0; - log_debug("New user %s logged in.", u->name); - - /* Make XDG_RUNTIME_DIR */ - r = user_mkdir_runtime_path(u); - if (r < 0) - return r; + /* + * If u->stopping is set, the user is marked for removal and the slice + * and service stop-jobs are queued. We have to clear that flag before + * queing the start-jobs again. If they succeed, the user object can be + * re-used just fine (pid1 takes care of job-ordering and proper + * restart), but if they fail, we want to force another user_stop() so + * possibly pending units are stopped. + * Note that we don't clear u->started, as we have no clue what state + * the user is in on failure here. Hence, we pretend the user is + * running so it will be properly taken down by GC. However, we clearly + * return an error from user_start() in that case, so no further + * reference to the user is taken. + */ + u->stopping = false; + + if (!u->started) { + log_debug("New user %s logged in.", u->name); + + /* Make XDG_RUNTIME_DIR */ + r = user_mkdir_runtime_path(u); + if (r < 0) + return r; + } /* Create cgroup */ r = user_start_slice(u); @@ -507,16 +524,16 @@ int user_start(User *u) { if (r < 0) return r; - if (!dual_timestamp_is_set(&u->timestamp)) - dual_timestamp_get(&u->timestamp); - - u->started = true; + if (!u->started) { + if (!dual_timestamp_is_set(&u->timestamp)) + dual_timestamp_get(&u->timestamp); + user_send_signal(u, true); + u->started = true; + } /* Save new user data */ user_save(u); - user_send_signal(u, true); - return 0; } -- 2.30.2