Attachment 46159 Details for Bug 36686 – Fix pam su exit error

[patch] Fix pam su exit error

0001-Fix-su-exit-by-moving-items-down-in-cgroups-while-clos.txt (text/plain), 11.70 KB, created by Alban Browaeys on 2011-04-29 09:54:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alban Browaeys

Created: 2011-04-29 09:54:43 UTC

Size: 11.70 KB

patch

obsolete

>From 2cb94ca99ac6f8e5edd5d63a165893bce6c64e82 Mon Sep 17 00:00:00 2001
>From: Alban Browaeys <prahal@yahoo.com>
>Date: Fri, 29 Apr 2011 18:06:20 +0200
>Subject: [PATCH] Fix su exit by moving items down in cgroups while closing.
>
>---
> src/pam-module.c |  117 +++++++++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 95 insertions(+), 22 deletions(-)
>
>diff --git a/src/pam-module.c b/src/pam-module.c
>index 93eb929..cd87213 100644
>--- a/src/pam-module.c
>+++ b/src/pam-module.c
>@@ -185,12 +185,13 @@ static int parse_argv(pam_handle_t *handle,
>         return 0;
> }
> 
>-static int open_file_and_lock(const char *fn) {
>+static int open_file_and_lock(const char *fn,
>+                struct passwd *pw) {
>         int fd;
> 
>         assert(fn);
> 
>-        if ((fd = open(fn, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW|O_CREAT, 0600)) < 0)
>+        if ((fd = open(fn, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW|O_CREAT, 0666)) < 0)
>                 return -errno;
> 
>         /* The BSD socket semantics are a lot nicer than those of
>@@ -199,6 +200,11 @@ static int open_file_and_lock(const char *fn) {
>          * as the filesystems in question should be local, and only
>          * locally accessible, and most likely even tmpfs. */
> 
>+        if (chmod_and_chown (fn, 0600, pw->pw_uid, pw->pw_gid) < 0) {
>+                close_nointr_nofail(fd);
>+                return -errno;
>+	}
>+
>         if (flock(fd, LOCK_EX) < 0) {
>                 close_nointr_nofail(fd);
>                 return -errno;
>@@ -213,7 +219,8 @@ enum {
>         SESSION_ID_RANDOM = 'r'
> };
> 
>-static uint64_t get_session_id(int *mode) {
>+static uint64_t get_session_id(int *mode,
>+                struct passwd *pw) {
>         char *s;
>         int fd;
> 
>@@ -236,7 +243,7 @@ static uint64_t get_session_id(int *mode) {
>                 }
> 
>         /* Second attempt, use our own counter. */
>-        if ((fd = open_file_and_lock(RUNTIME_DIR "/user/.pam-systemd-session")) >= 0) {
>+        if ((fd = open_file_and_lock(RUNTIME_DIR "/user/.pam-systemd-session", pw)) >= 0) {
>                 uint64_t counter;
>                 ssize_t r;
> 
>@@ -336,23 +343,72 @@ static int get_user_data(
>         return PAM_SUCCESS;
> }
> 
>+static int chmod_and_chown_intermediate_dir(const char *base_dir, const char *path, mode_t mode, uid_t uid, gid_t gid) {
>+        const char *p, *e;
>+        int r;
>+
>+        assert(base_dir);
>+        assert(path);
>+
>+        r = cg_set_task_access(SYSTEMD_CGROUP_CONTROLLER, base_dir, mode, uid, gid);
>+        if (r < 0)
>+                return -errno;
>+
>+        p = path + strspn(path, "/");
>+        for (;;) {
>+                char *buf = NULL;
>+                char *t;
>+
>+                e = p + strcspn(p, "/");
>+                p = e + strspn(e, "/");
>+
>+                /* Is this the last component? If so, then we're
>+                 * done */
>+                if (*p == 0)
>+                        return 0;
>+
>+                if (!(t = strndup(path, e - path)))
>+                        return -ENOMEM;
>+
>+                free(buf);
>+                r = asprintf(&buf, "%s/%s", base_dir, t);
>+                free(t);
>+
>+                if (r < 0)
>+                        return -errno;
>+
>+                r = cg_set_group_access(SYSTEMD_CGROUP_CONTROLLER, buf, mode, uid, gid);
>+                free(buf);
>+
>+                if (r < 0)
>+                        return -errno;
>+        }
>+
>+}
>+
> static int create_user_group(
>                 pam_handle_t *handle,
>                 const char *controller,
>-                const char *group,
>+                const char *group_basedir,
>+                const char *group_path,
>                 struct passwd *pw,
>                 bool attach,
>                 bool remember) {
> 
>+        char *buf = NULL;
>         int r;
> 
>         assert(handle);
>-        assert(group);
>+        assert(group_basedir);
>+        assert(group_path);
>+
>+        if (asprintf(&buf, "%s/%s", group_basedir, group_path) < 0)
>+                return PAM_BUF_ERR;
> 
>         if (attach)
>-                r = cg_create_and_attach(controller, group, 0);
>+                r = cg_create_and_attach(controller, buf, 0);
>         else
>-                r = cg_create(controller, group);
>+                r = cg_create(controller, buf);
> 
>         if (r < 0) {
>                 pam_syslog(handle, LOG_ERR, "Failed to create cgroup: %s", strerror(-r));
>@@ -370,8 +426,10 @@ static int create_user_group(
>                 }
>         }
> 
>-        if ((r = cg_set_task_access(controller, group, 0644, pw->pw_uid, pw->pw_gid)) < 0 ||
>-            (r = cg_set_group_access(controller, group, 0755, pw->pw_uid, pw->pw_gid)) < 0) {
>+        //if ((r = cg_set_group_access(SYSTEMD_CGROUP_CONTROLLER, group_basedir, 0777, pw->pw_uid, pw->pw_gid)) < 0 ||
>+        if ((r = cg_set_task_access(controller, buf, 0644, pw->pw_uid, pw->pw_gid)) < 0 ||
>+            (r = cg_set_group_access(controller, buf, 0755, pw->pw_uid, pw->pw_gid)) < 0 ||
>+            (r = chmod_and_chown_intermediate_dir(group_basedir, group_path, 0755, pw->pw_uid, pw->pw_gid)) < 0) {
>                 pam_syslog(handle, LOG_ERR, "Failed to change access modes: %s", strerror(-r));
>                 return PAM_SESSION_ERR;
>         }
>@@ -403,7 +461,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
>         const char *username = NULL;
>         struct passwd *pw;
>         int r;
>-        char *buf = NULL;
>+        char *buf = NULL, *user_path = NULL;
>         int lock_fd = -1;
>         bool create_session = true;
>         char **controllers = NULL, **reset_controllers = NULL, **c;
>@@ -433,13 +491,21 @@ _public_ PAM_EXTERN int pam_sm_open_session(
>                 goto finish;
>         }
> 
>-        if (safe_mkdir(RUNTIME_DIR "/user", 0755, 0, 0) < 0) {
>+        r = safe_mkdir(RUNTIME_DIR "/user", 0755, 0, 0);
>+        if (r < 0 && r != -EEXIST) {
>                 pam_syslog(handle, LOG_ERR, "Failed to create runtime directory: %m");
>                 r = PAM_SYSTEM_ERR;
>                 goto finish;
>         }
> 
>-        if ((lock_fd = open_file_and_lock(RUNTIME_DIR "/user/.pam-systemd-lock")) < 0) {
>+        if (chmod_and_chown (RUNTIME_DIR "/user", 0777, 0, 0) < 0){
>+                pam_syslog(handle, LOG_ERR, "Failed to set access mode on runtime directory: %m");
>+                r = PAM_SYSTEM_ERR;
>+                goto finish;
>+        }
>+
>+
>+        if ((lock_fd = open_file_and_lock(RUNTIME_DIR "/user/.pam-systemd-lock", pw)) < 0) {
>                 pam_syslog(handle, LOG_ERR, "Failed to lock runtime directory: %m");
>                 r = PAM_SYSTEM_ERR;
>                 goto finish;
>@@ -471,7 +537,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
>                 if (!(id = pam_getenv(handle, "XDG_SESSION_ID"))) {
>                         int mode;
> 
>-                        if (asprintf(&buf, "%llux", (unsigned long long) get_session_id(&mode)) < 0) {
>+                        if (asprintf(&buf, "%llux", (unsigned long long) get_session_id(&mode, pw)) < 0) {
>                                 r = PAM_BUF_ERR;
>                                 goto finish;
>                         }
>@@ -496,9 +562,9 @@ _public_ PAM_EXTERN int pam_sm_open_session(
>                         }
>                 }
> 
>-                r = asprintf(&buf, "%s/%s/%s", cgroup_user_tree, username, id);
>+                r = asprintf(&user_path, "%s/%s", username, id);
>         } else
>-                r = asprintf(&buf, "%s/%s/master", cgroup_user_tree, username);
>+                r = asprintf(&user_path, "%s/master", username);
> 
>         if (r < 0) {
>                 r = PAM_BUF_ERR;
>@@ -507,13 +573,13 @@ _public_ PAM_EXTERN int pam_sm_open_session(
> 
>         pam_syslog(handle, LOG_INFO, "Moving new user session for %s into control group %s.", username, buf);
> 
>-        if ((r = create_user_group(handle, SYSTEMD_CGROUP_CONTROLLER, buf, pw, true, true)) != PAM_SUCCESS)
>+        if ((r = create_user_group(handle, SYSTEMD_CGROUP_CONTROLLER, cgroup_user_tree, user_path, pw, true, true)) != PAM_SUCCESS)
>                 goto finish;
> 
>         /* The additional controllers don't really matter, so we
>          * ignore the return value */
>         STRV_FOREACH(c, controllers)
>-                create_user_group(handle, *c, buf, pw, true, false);
>+                create_user_group(handle, *c, cgroup_user_tree, user_path, pw, true, false);
> 
>         STRV_FOREACH(c, reset_controllers)
>                 reset_group(handle, *c);
>@@ -522,6 +588,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
> 
> finish:
>         free(buf);
>+        free(user_path);
> 
>         if (lock_fd >= 0)
>                 close_nointr_nofail(lock_fd);
>@@ -617,7 +684,7 @@ _public_ PAM_EXTERN int pam_sm_close_session(
>         bool kill_session = false;
>         bool kill_user = false;
>         int lock_fd = -1, r;
>-        char *session_path = NULL, *nosession_path = NULL, *user_path = NULL;
>+        char *session_path = NULL, *nosession_path = NULL, *user_path = NULL, *nosession_path_intermediate = NULL;
>         const char *id;
>         struct passwd *pw;
>         const void *created = NULL;
>@@ -646,7 +713,7 @@ _public_ PAM_EXTERN int pam_sm_close_session(
>                 goto finish;
>         }
> 
>-        if ((lock_fd = open_file_and_lock(RUNTIME_DIR "/user/.pam-systemd-lock")) < 0) {
>+        if ((lock_fd = open_file_and_lock(RUNTIME_DIR "/user/.pam-systemd-lock", pw)) < 0) {
>                 pam_syslog(handle, LOG_ERR, "Failed to lock runtime directory: %m");
>                 r = PAM_SYSTEM_ERR;
>                 goto finish;
>@@ -670,10 +737,15 @@ _public_ PAM_EXTERN int pam_sm_close_session(
>         if ((id = pam_getenv(handle, "XDG_SESSION_ID")) && created) {
> 
>                 if (asprintf(&session_path, "%s/%s/%s", cgroup_user_tree, username, id) < 0 ||
>+                    asprintf(&nosession_path_intermediate, "%s/master", username) < 0 ||
>                     asprintf(&nosession_path, "%s/%s/master", cgroup_user_tree, username) < 0) {
>                         r = PAM_BUF_ERR;
>                         goto finish;
>                 }
>+                if ((r = cg_attach(SYSTEMD_CGROUP_CONTROLLER, session_path, 0)) < 0)
>+                    pam_syslog(handle, LOG_ERR, "Failed to move us away: %s", strerror(-r));
>+
>+
> 
>                 if (kill_session && check_user_lists(handle, pw->pw_uid, kill_only_users, kill_exclude_users))  {
>                         pam_syslog(handle, LOG_INFO, "Killing remaining processes of user session %s of %s.", id, username);
>@@ -688,14 +760,14 @@ _public_ PAM_EXTERN int pam_sm_close_session(
>                          * cgroup. First, try to create the user group
>                          * in case it doesn't exist yet. Also, delete
>                          * the session group. */
>-                        create_user_group(handle, SYSTEMD_CGROUP_CONTROLLER, nosession_path, pw, false, false);
>+                        create_user_group(handle, SYSTEMD_CGROUP_CONTROLLER, cgroup_user_tree, nosession_path_intermediate, pw, false, false);
> 
>                         if ((r = cg_migrate_recursive(SYSTEMD_CGROUP_CONTROLLER, session_path, nosession_path, false, true)) < 0)
>                                 pam_syslog(handle, LOG_ERR, "Failed to migrate session cgroup: %s", strerror(-r));
>                 }
> 
>                 STRV_FOREACH(c, controllers) {
>-                        create_user_group(handle, *c, nosession_path, pw, false, false);
>+                        create_user_group(handle, *c, cgroup_user_tree, nosession_path_intermediate, pw, false, false);
> 
>                         if ((r = cg_migrate_recursive(*c, session_path, nosession_path, false, true)) < 0)
>                                 pam_syslog(handle, LOG_ERR, "Failed to migrate session cgroup in hierarchy %s: %s", *c, strerror(-r));
>@@ -750,6 +822,7 @@ finish:
> 
>         free(session_path);
>         free(nosession_path);
>+        free(nosession_path_intermediate);
>         free(user_path);
> 
>         strv_free(controllers);
>-- 
>1.7.5
>

Actions: View | Splinter Review

Attachments on bug 36686: 46159

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.