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/<pid>/cwd and /proc/<pid>/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 <hugh@veritas.com>
Cc: stable@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/fs/proc/base.c b/fs/proc/base.c
index beaa0ce3b82e1de8fec8941584a03f3dc32a700b..d1043613899c4a04787760dac730cf96a4c6858e 100644 (file)
--- 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)