lkml.org 
[lkml]   [2009]   [Feb]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: BUG: setuid sometimes doesn't.
On Thu, 26 Feb 2009, Joe Malicki wrote:
> ----- "Joe Malicki" <jmalicki@metacarta.com> wrote:
>
> > Very rarely, we experience a setuid program not properly getting
> > the euid of its owner.
> >
> > Thus far, we have only seen failures for the program being setuid
> > root, being run by a non-root user, on a multi-core machine. Trying
> > to
> > setuid to a user from root, *or* booting with maxcpus=1 and trying to
> > setuid from a non-root user to root, both fail.
>
> Sorry, misstated that.
>
> setuid from nonroot->root, or with maxcpus=1, always seems to work.
>
> Only multiple cores with setuid to root has failed for us.

Here's a shot in the dark: I may be misreading things, and I don't
quite see how it fits with the finer details you mention here; but
it looks to me as if /proc/*/cwd and /proc/*/root lookup interferes
with the fs->count check in fs/exec.c's unsafe_exec().

If you would, please give this patch against 2.6.28* a try (applies
to 2.6.29-rc too, but not to 2.6.24*), to see if it makes any
difference to you. I'm hoping not to hear from you for a while!

(I assume it's okay to read_lock fs->lock while holding task_lock:
I didn't see anywhere else doing so, but lockdep hasn't objected yet.)

Hugh

--- 2.6.28/fs/proc/base.c 2008-12-24 23:26:37.000000000 +0000
+++ linux/fs/proc/base.c 2009-02-26 15:39:00.000000000 +0000
@@ -148,15 +148,22 @@ static unsigned int pid_entry_count_dirs
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)
@@ -174,42 +181,24 @@ static int get_nr_threads(struct task_st
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;
}

@@ -567,7 +556,6 @@ static int mounts_open_common(struct ino
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;
@@ -581,22 +569,16 @@ static int mounts_open_common(struct ino
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)

\
 
 \ /
  Last update: 2009-02-26 17:11    [W:0.064 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site