[systemd-devel] [PATCH v2 0/7] logind: close races on user and session states

[Djalal Harouni]

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.