fix setuid sometimes doesn't (e426b64c) · Commits · 戴 / test

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@metacarta.com> Narrowed-down-by:

Michael Itz <mitz@metacarta.com> Tested-by:

Joe Malicki <jmalicki@metacarta.com> Signed-off-by:

Hugh Dickins <hugh@veritas.com> Cc: stable@kernel.org Signed-off-by:

Linus Torvalds <torvalds@linux-foundation.org>

fs/compat.c

+1 −1

Original line number	Diff line number	Diff line
		@@ -1441,7 +1441,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);

fs/exec.c

+3 −7

Original line number	Diff line number	Diff line
		@@ -1056,28 +1056,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;

		@@ -1300,7 +1296,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);

fs/internal.h

+1 −1

Original line number	Diff line number	Diff line
		@@ -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

Admin message