Bug 67288

Summary:	console login no longer possible when PAM_SESSION_ERR condition is triggered
Product:	systemd	Reporter:	Michael Biebl <mbiebl>
Component:	general	Assignee:	systemd-bugs
Status:	RESOLVED FIXED	QA Contact:	systemd-bugs
Severity:	major
Priority:	medium	CC:	michael+freedesktop
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	simulate error condition in pam_systemd

Description Michael Biebl 2013-07-25 03:16:29 UTC

Created attachment 82975 [details]
simulate error condition in pam_systemd

Version: 204
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=717772

While investigating [1], I noticed, that when an error condition occurs and pam_systemd returns  PAM_SESSION_ERR, I am no longer able to log in on the console. I'm immediately thrown back to the console. In the journal, I get the following log messages:

> Jul 20 14:08:08 pluto login[3476]: pam_unix(login:session): session opened for user michael by LOGIN(uid=0)
> Jul 20 14:08:08 pluto systemd-logind[5664]: New session 13 of user michael.
> Jul 20 14:08:08 pluto login[3476]: pam_systemd(login:session): Failed to parse message: Message has only 5 arguments, but more were expected
> Jul 20 14:08:08 pluto systemd-logind[5664]: Removed session 13.
> Jul 20 14:08:08 pluto systemd[1]: getty@tty2.service holdoff time over, scheduling restart.
> Jul 20 14:08:08 pluto systemd[1]: Stopping Getty on tty2...
> Jul 20 14:08:08 pluto systemd[1]: Starting Getty on tty2...
> Jul 20 14:08:08 pluto systemd[1]: Started Getty on tty2.

I initially noticed this behaviour, when there was version mismatch (libpam-systemd v204 talking to logind v44). But this is a general problem, since there are various places in the code, where an error condition can occur.
To simplify this, I wrote a tiny patch, which simulates an error condition in pam_sm_open_session(). If I apply that on top of v204, I'm also able to reproduce this behaviour on a Fedora 19 box.

The pam configuration has 
 session  optional pam_systemd.so

A failing pam_systemd shouldn't cause the complete PAM stack to fail.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=67131

Comment 1 Kay Sievers 2013-07-25 21:53:11 UTC

Fedora has a leading '-', doesn't that make a difference?
-session     optional      pam_systemd.so

Comment 2 Michael Biebl 2013-07-25 22:29:47 UTC

(In reply to comment #1)
> Fedora has a leading '-', doesn't that make a difference?
> -session     optional      pam_systemd.so

No, doesn't make a difference. Aside from the fact, that "-" is a Fedora specific, afaik what it does is ignore errors if the .so itself does not exist.

Comment 3 Michael Stapelberg 2013-07-26 20:39:55 UTC

Some debugging revealed: With mbiebl’s patch (http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=15;filename=pam-systemd-session-err.patch;att=1;bug=717772 ), you can still log in when removing these two lines:

        if (session_fd >= 0)
                close_nointr_nofail(session_fd);

Therefore, we think what happens is that logind creates the session, pam_systemd fails to parse the reply, closes the fd and logind kills the session, leading to that process being killed and the login terminating.

To double-check, I used pam_debug in this way:

session optional pam_debug.so open_session=session_err

Which works fine. It’s not the return code of the module, it’s the close() of the file descriptor.

I propose changing the code so that it won’t close the file descriptor at all. Even though sessions will pile up and not work as expected, you will still be able to log in, which is important. What do you think?

Comment 4 Michael Biebl 2013-07-26 20:58:40 UTC

(In reply to comment #3)
> I propose changing the code so that it won’t close the file descriptor at
> all. Even though sessions will pile up and not work as expected, you will
> still be able to log in, which is important. What do you think?

Does logind provide an API which allows one to tear down a session without killing all its processes?

Comment 5 Michael Stapelberg 2013-07-28 10:39:32 UTC

(In reply to comment #4)
> (In reply to comment #3)
> > I propose changing the code so that it won’t close the file descriptor at
> > all. Even though sessions will pile up and not work as expected, you will
> > still be able to log in, which is important. What do you think?
> 
> Does logind provide an API which allows one to tear down a session without
> killing all its processes?
From looking at http://www.freedesktop.org/wiki/Software/systemd/logind/ I would say no, it does not.

Also note that the code path we are talking about is entered (amongst other conditions) when the code cannot talk to logind, so depending on logind for the error handling seems wrong :).

Lennart, Kay, what do you think with regards to comment #3?

Comment 6 Lennart Poettering 2016-06-07 10:48:51 UTC

This was fixed quite some time back. Closing.

Comment 7 Michael Biebl 2016-06-07 10:50:36 UTC

(In reply to Lennart Poettering from comment #6)
> This was fixed quite some time back. Closing.

Which version/commit fixed this?

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.