From 6560e7a44e83e8dd111c64e3ed77ad08d2550e23 Mon Sep 17 00:00:00 2001 From: Hugh Dickins Date: Sat, 28 Mar 2009 23:21:27 +0000 Subject: [PATCH] fix setuid sometimes wouldn't commit 7c2c7d993044cddc5010f6f429b100c63bc7dffb upstream. check_unsafe_exec() also notes whether the fs_struct is being shared by more threads than will get killed by the exec, and if so sets LSM_UNSAFE_SHARE to make bprm_set_creds() careful about euid. But /proc//cwd and /proc//root lookups make transient use of get_fs_struct(), which also raises that sharing count. This might occasionally cause a setuid program not to change euid, in the same way as happened with files->count (check_unsafe_exec also looks at sighand->count, but /proc doesn't raise that one). We'd prefer exec not to unshare fs_struct: so fix this in procfs, replacing get_fs_struct() by get_fs_path(), which does path_get while still holding task_lock, instead of raising fs->count. Signed-off-by: Hugh Dickins Cc: stable@kernel.org Signed-off-by: Greg Kroah-Hartman --- fs/proc/base.c | 50 ++++++++++++++++---------------------------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index beaa0ce3b82e..d1043613899c 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -146,15 +146,22 @@ static unsigned int pid_entry_count_dirs(const struct pid_entry *entries, return count; } -static struct fs_struct *get_fs_struct(struct task_struct *task) +static int get_fs_path(struct task_struct *task, struct path *path, bool root) { struct fs_struct *fs; + int result = -ENOENT; + task_lock(task); fs = task->fs; - if(fs) - atomic_inc(&fs->count); + if (fs) { + read_lock(&fs->lock); + *path = root ? fs->root : fs->pwd; + path_get(path); + read_unlock(&fs->lock); + result = 0; + } task_unlock(task); - return fs; + return result; } static int get_nr_threads(struct task_struct *tsk) @@ -172,42 +179,24 @@ static int get_nr_threads(struct task_struct *tsk) static int proc_cwd_link(struct inode *inode, struct path *path) { struct task_struct *task = get_proc_task(inode); - struct fs_struct *fs = NULL; int result = -ENOENT; if (task) { - fs = get_fs_struct(task); + result = get_fs_path(task, path, 0); put_task_struct(task); } - if (fs) { - read_lock(&fs->lock); - *path = fs->pwd; - path_get(&fs->pwd); - read_unlock(&fs->lock); - result = 0; - put_fs_struct(fs); - } return result; } static int proc_root_link(struct inode *inode, struct path *path) { struct task_struct *task = get_proc_task(inode); - struct fs_struct *fs = NULL; int result = -ENOENT; if (task) { - fs = get_fs_struct(task); + result = get_fs_path(task, path, 1); put_task_struct(task); } - if (fs) { - read_lock(&fs->lock); - *path = fs->root; - path_get(&fs->root); - read_unlock(&fs->lock); - result = 0; - put_fs_struct(fs); - } return result; } @@ -596,7 +585,6 @@ static int mounts_open_common(struct inode *inode, struct file *file, struct task_struct *task = get_proc_task(inode); struct nsproxy *nsp; struct mnt_namespace *ns = NULL; - struct fs_struct *fs = NULL; struct path root; struct proc_mounts *p; int ret = -EINVAL; @@ -610,22 +598,16 @@ static int mounts_open_common(struct inode *inode, struct file *file, get_mnt_ns(ns); } rcu_read_unlock(); - if (ns) - fs = get_fs_struct(task); + if (ns && get_fs_path(task, &root, 1) == 0) + ret = 0; put_task_struct(task); } if (!ns) goto err; - if (!fs) + if (ret) goto err_put_ns; - read_lock(&fs->lock); - root = fs->root; - path_get(&root); - read_unlock(&fs->lock); - put_fs_struct(fs); - ret = -ENOMEM; p = kmalloc(sizeof(struct proc_mounts), GFP_KERNEL); if (!p) -- 2.39.5