Joe Malicki reports that setuid sometimes doesn't: very rarely,
a setuid root program does not get root euid; and, by the way,
they have a health check running lsof every few minutes.
Right, check_unsafe_exec() notes whether the files_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>/fd and /proc/<pid>/fdinfo lookups make transient
use of get_files_struct(), which also raises that sharing count.
There's a rather simple fix for this: exec's check on files->count
has been redundant ever since 2.6.1 made it unshare_files() (except
while compat_do_execve() omitted to do so) - just remove that check.
[Note to -stable: this patch will not apply before 2.6.29: earlier
releases should just remove the files->count line from unsafe_exec().]
Reported-by: Joe Malicki <jmalicki@xxxxxxxxxxxxx>
Narrowed-down-by: Michael Itz <mitz@xxxxxxxxxxxxx>
Tested-by: Joe Malicki <jmalicki@xxxxxxxxxxxxx>
Signed-off-by: Hugh Dickins <hugh@xxxxxxxxxxx>
Cc: stable@xxxxxxxxxx
---
fs/compat.c | 2 +-
fs/exec.c | 10 +++-------
fs/internal.h | 2 +-
3 files changed, 5 insertions(+), 9 deletions(-)
--- 2.6.29/fs/compat.c 2009-03-23 23:12:14.000000000 +0000
+++ linux/fs/compat.c 2009-03-28 18:06:02.000000000 +0000
@@ -1412,7 +1412,7 @@ int compat_do_execve(char * filename,
bprm->cred = prepare_exec_creds();
if (!bprm->cred)
goto out_unlock;
- check_unsafe_exec(bprm, current->files);
+ check_unsafe_exec(bprm);
file = open_exec(filename);
retval = PTR_ERR(file);
--- 2.6.29/fs/exec.c 2009-03-23 23:12:14.000000000 +0000
+++ linux/fs/exec.c 2009-03-28 18:06:02.000000000 +0000
@@ -1049,28 +1049,24 @@ EXPORT_SYMBOL(install_exec_creds);
* - the caller must hold current->cred_exec_mutex to protect against
* PTRACE_ATTACH
*/
-void check_unsafe_exec(struct linux_binprm *bprm, struct files_struct *files)
+void check_unsafe_exec(struct linux_binprm *bprm)
{
struct task_struct *p = current, *t;
unsigned long flags;
- unsigned n_fs, n_files, n_sighand;
+ unsigned n_fs, n_sighand;
bprm->unsafe = tracehook_unsafe_exec(p);
n_fs = 1;
- n_files = 1;
n_sighand = 1;
lock_task_sighand(p, &flags);
for (t = next_thread(p); t != p; t = next_thread(t)) {
if (t->fs == p->fs)
n_fs++;
- if (t->files == files)
- n_files++;
n_sighand++;
}
if (atomic_read(&p->fs->count) > n_fs ||
- atomic_read(&p->files->count) > n_files ||
atomic_read(&p->sighand->count) > n_sighand)
bprm->unsafe |= LSM_UNSAFE_SHARE;
@@ -1289,7 +1285,7 @@ int do_execve(char * filename,
bprm->cred = prepare_exec_creds();
if (!bprm->cred)
goto out_unlock;
- check_unsafe_exec(bprm, displaced);
+ check_unsafe_exec(bprm);
file = open_exec(filename);
retval = PTR_ERR(file);
--- 2.6.29/fs/internal.h 2009-03-23 23:12:14.000000000 +0000
+++ linux/fs/internal.h 2009-03-28 18:06:02.000000000 +0000
@@ -43,7 +43,7 @@ extern void __init chrdev_init(void);
/*
* exec.c
*/
-extern void check_unsafe_exec(struct linux_binprm *, struct files_struct *);
+extern void check_unsafe_exec(struct linux_binprm *);
/*
* namespace.c
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
