Re: [systemd-devel] [PATCH v2 0/7] logind: close races on user and session states
On Sat, Feb 08, 2014 at 05:00:57PM +0100, Djalal Harouni wrote: On Sat, Feb 08, 2014 at 12:39:25AM +0100, Zbigniew Jędrzejewski-Szmek wrote: On Thu, Feb 06, 2014 at 09:37:13PM +0100, Djalal Harouni wrote: Summary: Currently logind will not clear sessions on logout. The bug is confirmed for getty and ssh logins. This series is preparation for next patches to address that bug, it does not fix it. However, this series also fixe real races on user and session states. This ensures that user_save() and session_save() functions will write the correct user and session state to the state files. The logind bug was already discussed here: http://lists.freedesktop.org/archives/systemd-devel/2014-January/015968.html So, I did some testing with current git (ba978d7). And this bug appears to be only partially gone. After my second user logs out, loginctl stops showing the session. But the manager *remains*. I confirm this, I've identified the problem, please see below. Hm, the manager remains, but /run/user/uid/systemd/ gets nuked. So when the user logins in a second time, communication with user manager is broken. I also confirm, I didn't have time to debug this one. Ok this one was also fixed by the same patch: http://lists.freedesktop.org/archives/systemd-devel/2014-February/016754.html I guess, the runtime dir will be removed on logout in user_finalize(), but user_stop() was never called so the service is still active. During next loggin, the service unit which is still active will prevent creating the private dbus socket... anyway it works now, we'll see what Lennart thinks. -- Djalal Harouni http://opendz.org ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2 0/7] logind: close races on user and session states
On Sat, Feb 08, 2014 at 12:39:25AM +0100, Zbigniew Jędrzejewski-Szmek wrote: On Thu, Feb 06, 2014 at 09:37:13PM +0100, Djalal Harouni wrote: Summary: Currently logind will not clear sessions on logout. The bug is confirmed for getty and ssh logins. This series is preparation for next patches to address that bug, it does not fix it. However, this series also fixe real races on user and session states. This ensures that user_save() and session_save() functions will write the correct user and session state to the state files. The logind bug was already discussed here: http://lists.freedesktop.org/archives/systemd-devel/2014-January/015968.html So, I did some testing with current git (ba978d7). And this bug appears to be only partially gone. After my second user logs out, loginctl stops showing the session. But the manager *remains*. I confirm this, I've identified the problem, please see below. Hm, the manager remains, but /run/user/uid/systemd/ gets nuked. So when the user logins in a second time, communication with user manager is broken. I also confirm, I didn't have time to debug this one. Attaching strace to systemd --user during the orignial session close yields... nothing. Both systemd and (sd-pam) remain undisturbed... Yes sd-pam and the user instance will stay alive for *another* user after logout. In manager_gc() the user_stop() is never called. if (!user_check_gc(user, drop_not_started) user_get_state(user) != USER_CLOSING) user_stop(user); Note: with the recent commits, user_stop() will set user-stopping to say that user_stop() was called. But currently with this logic if the user_check_gc(...) returns false, this means that 'u-sessions' is NULL, so user_get_state(user) will for sure return USER_CLOSING if linger is not set and if it is not open state. I think that we should remove that USER_CLOSING check, will just test it. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2 0/7] logind: close races on user and session states
On Sat, Feb 08, 2014 at 12:39:25AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
systemd-logind[2998]: Failed to process message [type=signal sender=:1.1
path=/org/freedesktop/systemd1/job/3284
interface=org.freedesktop.DBus.Properties member=PropertiesChanged
signature=sa{sv}as]: Invalid argument
So, this happens because match_properties_changed() expects
path=/org/freedesktop/systemd1/unit/... but gets .../job/...
I'll push a patch to quietly ignore those.
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2 0/7] logind: close races on user and session states
On Thu, 06.02.14 21:37, Djalal Harouni (tix...@opendz.org) wrote: Heya! So, I was working on fixes for this in parallel which I have pushed earlier today, which change a couple of things around in more complex ways. Some of them conflict with yours changes. I'll commit the ones of yours that still appear necessary. If you think I am missing some, please let me know. The new logic I commited changes around how scopes are used by logind. Previously the KillUserProcesses= option in logind.conf would influence KillMode= of the scope. However that was a bad idea since we actually want to provide people a way to terminate sessions and we actually need them to order things properly on shutdown so that scopes go away before the network is removed, and suchlike. Hence I changed the logic so we are OK with leavig scopes around from the logind side, we just mark the session entries as closing. KillUserProcesses= hence simply controls whether we leave them around and hence the session in closing or whether we actively shut them down. Sorry for commiting into the middle of your work! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2 0/7] logind: close races on user and session states
On Fri, Feb 07, 2014 at 04:29:48PM +0100, Lennart Poettering wrote: On Thu, 06.02.14 21:37, Djalal Harouni (tix...@opendz.org) wrote: Heya! So, I was working on fixes for this in parallel which I have pushed earlier today, which change a couple of things around in more complex ways. Some of them conflict with yours changes. I'll commit the ones of yours that still appear necessary. If you think I am missing some, please let me know. Ok, I'll do. The new logic I commited changes around how scopes are used by logind. Previously the KillUserProcesses= option in logind.conf would influence KillMode= of the scope. However that was a bad idea since we actually want to provide people a way to terminate sessions and we actually need them to order things properly on shutdown so that scopes go away before the network is removed, and suchlike. Hence I changed the Yes, I was going to ask about why scopes are influenced by KillUserProcesses? anyway I'll read your changes. logic so we are OK with leavig scopes around from the logind side, we just mark the session entries as closing. KillUserProcesses= hence simply controls whether we leave them around and hence the session in closing or whether we actively shut them down. Ok. Sorry for commiting into the middle of your work! No worries, I did a dive into the internals and the picture is still not clear :-) Lennart Thanks -- Lennart Poettering, Red Hat -- Djalal Harouni http://opendz.org ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 0/7] logind: close races on user and session states
Summary: Currently logind will not clear sessions on logout. The bug is confirmed for getty and ssh logins. This series is preparation for next patches to address that bug, it does not fix it. However, this series also fixe real races on user and session states. This ensures that user_save() and session_save() functions will write the correct user and session state to the state files. The logind bug was already discussed here: http://lists.freedesktop.org/archives/systemd-devel/2014-January/015968.html Patches were proposed, but they failed to address the bug since there are other problems related to user and session states: http://lists.freedesktop.org/archives/systemd-devel/2014-January/016372.html A first version to fix these race conditions on user and sessions states was proposed: http://lists.freedesktop.org/archives/systemd-devel/2014-January/016373.html This series is a v2, it will close all the discovered races on user and session states. The commit logs will tell you the story of each case. Now as noted above, this series fix real races and in the same time it is needed to fix the logind bug where sessions are not cleaned after logouts. Proposed plan to clean logind closed sessions: 1) Make the user and session states stable (this series fix it). 2) Improve session_check_gc() and user_check_gc() to check if: the state is closing and the cgroup is empty. 3) Push session and user into the gc during logout, in pam_systemd method_release_session() Now I've a patch that implements 2) and 3) on top of 1), sometimes it will work and successfully clean closed sessions, and sometimes it will not. This is due to a race when we close the session and try to terminate session processes and in the same time we are trying to see if the cgroup is empty... which is another problem on its own. So here are the patches, please consider this series since it will fix real races and it will make sure that user_save() and session_save() will write the correct state to the state files. Patches 1,6,7 are code cleanup. Patches 2,3,4,5 close races on user and session states. Djalal Harouni (7): 0001 logind: add function session_jobs_reply() to unify the create reply unify shared code in a single function. 0002 logind: close races on user and session states during login 0003 logind: close races on session state at logout 0004 logind: close races on user state at logout close race conditions on user and session states. 0005 logind: just call session_get_state() to get the session state make user_get_state() consistent with session_get_state() 0006 logind: add user_is_opening() and session_is_opening() 0007 logind: do not call session_jobs_reply() on CLOSING make sure to not call session_send_create_reply() when jobs finish during closing state. src/login/logind-dbus.c | 87 +++ src/login/logind-session-dbus.c | 11 --- src/login/logind-session.c | 10 +- src/login/logind-session.h | 4 +++- src/login/logind-user.c | 20 +--- src/login/logind-user.h | 3 +++ 6 files changed, 99 insertions(+), 36 deletions(-) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
