Re: [systemd-devel] [PATCH] sd-pam: Drop uid so parent signal arrives at child.

[Lennart Poettering]

On Thu, 17.05.12 12:17, Auke Kok (auke-jan.h@intel.com) wrote:

 The PAM helper thread needs to capture the death signal from the
 parent, but is prohibited from doing so since when the child dies
 as normal user, the kernel won't allow it to send a TERM to the
 PAM helper thread which is running as root.
 
 This causes the PAM threads to never exit, accumulating after
 user sessions exit.
 
 There is however really no need to keep the PAM threads running as
 root, so, we can just setresuid() to the same user as defined in the
 unit file for the parent thread (User=). This makes the TERM signal
 arrive as normal. In case setresuid() fails, we ignore the error, so
 we at least fall back to the current behaviour.

Applied. In the long run we probably should reshuffle all of this so
that the pam stuff is moved outside of the systemd binary and is
implemented via an explicit wrapper tool. The fact that the pam thingy
currently is a child of the main process is kinda neat, but probably not
the best choice after all.

We shouldn't leave long running children of PID 1 around that have not
exec()'ed to another binary, since they might cause unnecessary
pagefaults in PID 1. hence the idea of splitting this into its own
binary.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] sd-pam: Drop uid so parent signal arrives at child.

[Auke Kok]

The PAM helper thread needs to capture the death signal from the
parent, but is prohibited from doing so since when the child dies
as normal user, the kernel won't allow it to send a TERM to the
PAM helper thread which is running as root.

This causes the PAM threads to never exit, accumulating after
user sessions exit.

There is however really no need to keep the PAM threads running as
root, so, we can just setresuid() to the same user as defined in the
unit file for the parent thread (User=). This makes the TERM signal
arrive as normal. In case setresuid() fails, we ignore the error, so
we at least fall back to the current behaviour.
---
 src/core/execute.c |   18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/core/execute.c b/src/core/execute.c
index 953cfa2..4d40919 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -703,6 +703,7 @@ static int null_conv(
 static int setup_pam(
 const char *name,
 const char *user,
+uid_t uid,
 const char *tty,
 char ***pam_env,
 int fds[], unsigned n_fds) {
@@ -781,10 +782,17 @@ static int setup_pam(
 open here that have been opened by PAM. */
 close_many(fds, n_fds);
 
-/* Wait until our parent died. This will most likely
- * not work since the kernel does not allow
- * unprivileged parents kill their privileged children
- * this way. We rely on the control groups kill logic
+/* Drop privileges - we don't need any to pam_close_session
+ * and this will make PR_SET_PDEATHSIG work in most cases.
+ * If this fails, ignore the error - but expect sd-pam threads
+ * to fail to exit normally */
+if (setresuid(uid, uid, uid)  0)
+log_error(Error: Failed to setresuid() in sd-pam: 
%s, strerror(-r));
+
+/* Wait until our parent died. This will only work if
+ * the above setresuid() succeeds, otherwise the kernel
+ * will not allow unprivileged parents kill their privileged
+ * children this way. We rely on the control groups kill logic
  * to do the rest for us. */
 if (prctl(PR_SET_PDEATHSIG, SIGTERM)  0)
 goto child_finish;
@@ -1294,7 +1302,7 @@ int exec_spawn(ExecCommand *command,
 
 #ifdef HAVE_PAM
 if (context-pam_name  username) {
-err = setup_pam(context-pam_name, username, 
context-tty_path, pam_env, fds, n_fds);
+err = setup_pam(context-pam_name, username, uid, 
context-tty_path, pam_env, fds, n_fds);
 if (err  0) {
 r = EXIT_PAM;
 goto fail_child;
-- 
1.7.10

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel