Commit b7fbe52c authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[PATCH] compute_creds race

From: Andy Lutomirski <luto@myrealbox.com>

Fixes from me, Olaf Dietsche <olaf+list.linux-kernel@olafdietsche.de>

In fs/exec.c, compute_creds does:

	task_lock(current);
	if (bprm->e_uid != current->uid || bprm->e_gid != current->gid) {
                 current->mm->dumpable = 0;

		if (must_not_trace_exec(current)
		    || atomic_read(&current->fs->count) > 1
		    || atomic_read(&current->files->count) > 1
		    || atomic_read(&current->sighand->count) > 1) {
			if(!capable(CAP_SETUID)) {
				bprm->e_uid = current->uid;
				bprm->e_gid = current->gid;
			}
		}
	}

         current->suid = current->euid = current->fsuid = bprm->e_uid;
         current->sgid = current->egid = current->fsgid = bprm->e_gid;

	task_unlock(current);

	security_bprm_compute_creds(bprm);

I assume the task_lock is to prevent another process (on SMP or preempt)
from ptracing the execing process between the check and the assignment.  If
that's the concern then the fact that the lock is dropped before the call
to security_brpm_compute_creds means that, if security_bprm_compute_creds
does anything interesting, there's a race.

For my (nearly complete) caps patch, I obviously need to fix this.  But I
think it may be exploitable now.  Suppose there are two processes, A (the
malicious code) and B (which uses exec).  B starts out unprivileged (A and
B have, e.g., uid and euid = 500).

1. A ptraces B.

2. B calls exec on some setuid-root program.

3. in cap_bprm_set_security, B sets bprm->cap_permitted to the full
   set.

4. B gets to compute_creds in exec.c, calls task_lock, and does not
   change its uid.

5. B calls task_unlock.

6. A detaches from B (on preempt or SMP).

7. B gets to task_lock in cap_bprm_compute_creds, changes its
   capabilities, and returns from compute_creds into load_elf_binary.

8. load_elf_binary calls create_elf_tables (line 852 in 2.6.5-mm1),
   which calls cap_bprm_secureexec (through LSM), which returns false (!).

9. exec finishes.

The setuid program is now running with uid=euid=500 but full permitted
capabilities.  There are two (or three) ways to effectively get local root
now:

1.  IIRC, linux 2.4 doesn't check capabilities in ptrace, so A could
   just ptrace B again.

2. LD_PRELOAD.

3.  There are probably programs that will misbehave on their own under
   these circumstances.

Is there some reason why this is not doable?

The patch renames bprm_compute_creds to bprm_apply_creds and moves all uid
logic into the hook, where the test and the resulting modification can both
happen under task_lock().

This way, out-of-tree LSMs will fail to compile instead of malfunctioning. 
It should also make life easier for LSMs and will certainly make it easier
for me to finish the cap patch.
parent ce0cbde1
No related merge requests found
...@@ -870,15 +870,6 @@ int flush_old_exec(struct linux_binprm * bprm) ...@@ -870,15 +870,6 @@ int flush_old_exec(struct linux_binprm * bprm)
EXPORT_SYMBOL(flush_old_exec); EXPORT_SYMBOL(flush_old_exec);
/*
* We mustn't allow tracing of suid binaries, unless
* the tracer has the capability to trace anything..
*/
static inline int must_not_trace_exec(struct task_struct * p)
{
return (p->ptrace & PT_PTRACED) && !(p->ptrace & PT_PTRACE_CAP);
}
/* /*
* Fill the binprm structure from the inode. * Fill the binprm structure from the inode.
* Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes * Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes
...@@ -945,27 +936,7 @@ EXPORT_SYMBOL(prepare_binprm); ...@@ -945,27 +936,7 @@ EXPORT_SYMBOL(prepare_binprm);
void compute_creds(struct linux_binprm *bprm) void compute_creds(struct linux_binprm *bprm)
{ {
task_lock(current); security_bprm_apply_creds(bprm);
if (bprm->e_uid != current->uid || bprm->e_gid != current->gid) {
current->mm->dumpable = 0;
if (must_not_trace_exec(current)
|| atomic_read(&current->fs->count) > 1
|| atomic_read(&current->files->count) > 1
|| atomic_read(&current->sighand->count) > 1) {
if(!capable(CAP_SETUID)) {
bprm->e_uid = current->uid;
bprm->e_gid = current->gid;
}
}
}
current->suid = current->euid = current->fsuid = bprm->e_uid;
current->sgid = current->egid = current->fsgid = bprm->e_gid;
task_unlock(current);
security_bprm_compute_creds(bprm);
} }
EXPORT_SYMBOL(compute_creds); EXPORT_SYMBOL(compute_creds);
......
...@@ -44,7 +44,7 @@ extern int cap_capget (struct task_struct *target, kernel_cap_t *effective, kern ...@@ -44,7 +44,7 @@ extern int cap_capget (struct task_struct *target, kernel_cap_t *effective, kern
extern int cap_capset_check (struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); extern int cap_capset_check (struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
extern void cap_capset_set (struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); extern void cap_capset_set (struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
extern int cap_bprm_set_security (struct linux_binprm *bprm); extern int cap_bprm_set_security (struct linux_binprm *bprm);
extern void cap_bprm_compute_creds (struct linux_binprm *bprm); extern void cap_bprm_apply_creds (struct linux_binprm *bprm);
extern int cap_bprm_secureexec(struct linux_binprm *bprm); extern int cap_bprm_secureexec(struct linux_binprm *bprm);
extern int cap_inode_setxattr(struct dentry *dentry, char *name, void *value, size_t size, int flags); extern int cap_inode_setxattr(struct dentry *dentry, char *name, void *value, size_t size, int flags);
extern int cap_inode_removexattr(struct dentry *dentry, char *name); extern int cap_inode_removexattr(struct dentry *dentry, char *name);
...@@ -102,7 +102,7 @@ struct swap_info_struct; ...@@ -102,7 +102,7 @@ struct swap_info_struct;
* @bprm_free_security: * @bprm_free_security:
* @bprm contains the linux_binprm structure to be modified. * @bprm contains the linux_binprm structure to be modified.
* Deallocate and clear the @bprm->security field. * Deallocate and clear the @bprm->security field.
* @bprm_compute_creds: * @bprm_apply_creds:
* Compute and set the security attributes of a process being transformed * Compute and set the security attributes of a process being transformed
* by an execve operation based on the old attributes (current->security) * by an execve operation based on the old attributes (current->security)
* and the information saved in @bprm->security by the set_security hook. * and the information saved in @bprm->security by the set_security hook.
...@@ -115,7 +115,7 @@ struct swap_info_struct; ...@@ -115,7 +115,7 @@ struct swap_info_struct;
* @bprm contains the linux_binprm structure. * @bprm contains the linux_binprm structure.
* @bprm_set_security: * @bprm_set_security:
* Save security information in the bprm->security field, typically based * Save security information in the bprm->security field, typically based
* on information about the bprm->file, for later use by the compute_creds * on information about the bprm->file, for later use by the apply_creds
* hook. This hook may also optionally check permissions (e.g. for * hook. This hook may also optionally check permissions (e.g. for
* transitions between security domains). * transitions between security domains).
* This hook may be called multiple times during a single execve, e.g. for * This hook may be called multiple times during a single execve, e.g. for
...@@ -924,7 +924,7 @@ struct swap_info_struct; ...@@ -924,7 +924,7 @@ struct swap_info_struct;
* Check permission before allowing the @parent process to trace the * Check permission before allowing the @parent process to trace the
* @child process. * @child process.
* Security modules may also want to perform a process tracing check * Security modules may also want to perform a process tracing check
* during an execve in the set_security or compute_creds hooks of * during an execve in the set_security or apply_creds hooks of
* binprm_security_ops if the process is being traced and its security * binprm_security_ops if the process is being traced and its security
* attributes would be changed by the execve. * attributes would be changed by the execve.
* @parent contains the task_struct structure for parent process. * @parent contains the task_struct structure for parent process.
...@@ -1026,7 +1026,7 @@ struct security_operations { ...@@ -1026,7 +1026,7 @@ struct security_operations {
int (*bprm_alloc_security) (struct linux_binprm * bprm); int (*bprm_alloc_security) (struct linux_binprm * bprm);
void (*bprm_free_security) (struct linux_binprm * bprm); void (*bprm_free_security) (struct linux_binprm * bprm);
void (*bprm_compute_creds) (struct linux_binprm * bprm); void (*bprm_apply_creds) (struct linux_binprm * bprm);
int (*bprm_set_security) (struct linux_binprm * bprm); int (*bprm_set_security) (struct linux_binprm * bprm);
int (*bprm_check_security) (struct linux_binprm * bprm); int (*bprm_check_security) (struct linux_binprm * bprm);
int (*bprm_secureexec) (struct linux_binprm * bprm); int (*bprm_secureexec) (struct linux_binprm * bprm);
...@@ -1290,9 +1290,9 @@ static inline void security_bprm_free (struct linux_binprm *bprm) ...@@ -1290,9 +1290,9 @@ static inline void security_bprm_free (struct linux_binprm *bprm)
{ {
security_ops->bprm_free_security (bprm); security_ops->bprm_free_security (bprm);
} }
static inline void security_bprm_compute_creds (struct linux_binprm *bprm) static inline void security_bprm_apply_creds (struct linux_binprm *bprm)
{ {
security_ops->bprm_compute_creds (bprm); security_ops->bprm_apply_creds (bprm);
} }
static inline int security_bprm_set (struct linux_binprm *bprm) static inline int security_bprm_set (struct linux_binprm *bprm)
{ {
...@@ -1962,9 +1962,9 @@ static inline int security_bprm_alloc (struct linux_binprm *bprm) ...@@ -1962,9 +1962,9 @@ static inline int security_bprm_alloc (struct linux_binprm *bprm)
static inline void security_bprm_free (struct linux_binprm *bprm) static inline void security_bprm_free (struct linux_binprm *bprm)
{ } { }
static inline void security_bprm_compute_creds (struct linux_binprm *bprm) static inline void security_bprm_apply_creds (struct linux_binprm *bprm)
{ {
cap_bprm_compute_creds (bprm); cap_bprm_apply_creds (bprm);
} }
static inline int security_bprm_set (struct linux_binprm *bprm) static inline int security_bprm_set (struct linux_binprm *bprm)
......
...@@ -35,7 +35,7 @@ static struct security_operations capability_ops = { ...@@ -35,7 +35,7 @@ static struct security_operations capability_ops = {
.netlink_send = cap_netlink_send, .netlink_send = cap_netlink_send,
.netlink_recv = cap_netlink_recv, .netlink_recv = cap_netlink_recv,
.bprm_compute_creds = cap_bprm_compute_creds, .bprm_apply_creds = cap_bprm_apply_creds,
.bprm_set_security = cap_bprm_set_security, .bprm_set_security = cap_bprm_set_security,
.bprm_secureexec = cap_bprm_secureexec, .bprm_secureexec = cap_bprm_secureexec,
......
...@@ -115,13 +115,15 @@ int cap_bprm_set_security (struct linux_binprm *bprm) ...@@ -115,13 +115,15 @@ int cap_bprm_set_security (struct linux_binprm *bprm)
return 0; return 0;
} }
/* Copied from fs/exec.c */
static inline int must_not_trace_exec (struct task_struct *p) static inline int must_not_trace_exec (struct task_struct *p)
{ {
return (p->ptrace & PT_PTRACED) && !(p->ptrace & PT_PTRACE_CAP); return ((p->ptrace & PT_PTRACED) && !(p->ptrace & PT_PTRACE_CAP))
|| atomic_read(&p->fs->count) > 1
|| atomic_read(&p->files->count) > 1
|| atomic_read(&p->sighand->count) > 1;
} }
void cap_bprm_compute_creds (struct linux_binprm *bprm) void cap_bprm_apply_creds (struct linux_binprm *bprm)
{ {
/* Derived from fs/exec.c:compute_creds. */ /* Derived from fs/exec.c:compute_creds. */
kernel_cap_t new_permitted, working; kernel_cap_t new_permitted, working;
...@@ -132,20 +134,28 @@ void cap_bprm_compute_creds (struct linux_binprm *bprm) ...@@ -132,20 +134,28 @@ void cap_bprm_compute_creds (struct linux_binprm *bprm)
new_permitted = cap_combine (new_permitted, working); new_permitted = cap_combine (new_permitted, working);
task_lock(current); task_lock(current);
if (bprm->e_uid != current->uid || bprm->e_gid != current->gid) {
current->mm->dumpable = 0;
if (must_not_trace_exec(current) && !capable(CAP_SETUID)) {
bprm->e_uid = current->uid;
bprm->e_gid = current->gid;
}
}
current->suid = current->euid = current->fsuid = bprm->e_uid;
current->sgid = current->egid = current->fsgid = bprm->e_gid;
if (!cap_issubset (new_permitted, current->cap_permitted)) { if (!cap_issubset (new_permitted, current->cap_permitted)) {
current->mm->dumpable = 0; current->mm->dumpable = 0;
if (must_not_trace_exec (current) if (must_not_trace_exec (current) && !capable (CAP_SETPCAP)) {
|| atomic_read (&current->fs->count) > 1
|| atomic_read (&current->files->count) > 1
|| atomic_read (&current->sighand->count) > 1) {
if (!capable (CAP_SETPCAP)) {
new_permitted = cap_intersect (new_permitted, new_permitted = cap_intersect (new_permitted,
current-> current->
cap_permitted); cap_permitted);
} }
} }
}
/* For init, we want to retain the capabilities set /* For init, we want to retain the capabilities set
* in the init_task struct. Thus we skip the usual * in the init_task struct. Thus we skip the usual
...@@ -377,7 +387,7 @@ EXPORT_SYMBOL(cap_capget); ...@@ -377,7 +387,7 @@ EXPORT_SYMBOL(cap_capget);
EXPORT_SYMBOL(cap_capset_check); EXPORT_SYMBOL(cap_capset_check);
EXPORT_SYMBOL(cap_capset_set); EXPORT_SYMBOL(cap_capset_set);
EXPORT_SYMBOL(cap_bprm_set_security); EXPORT_SYMBOL(cap_bprm_set_security);
EXPORT_SYMBOL(cap_bprm_compute_creds); EXPORT_SYMBOL(cap_bprm_apply_creds);
EXPORT_SYMBOL(cap_bprm_secureexec); EXPORT_SYMBOL(cap_bprm_secureexec);
EXPORT_SYMBOL(cap_inode_setxattr); EXPORT_SYMBOL(cap_inode_setxattr);
EXPORT_SYMBOL(cap_inode_removexattr); EXPORT_SYMBOL(cap_inode_removexattr);
......
...@@ -26,6 +26,8 @@ ...@@ -26,6 +26,8 @@
#include <net/sock.h> #include <net/sock.h>
#include <linux/xattr.h> #include <linux/xattr.h>
#include <linux/hugetlb.h> #include <linux/hugetlb.h>
#include <linux/ptrace.h>
#include <linux/file.h>
static int dummy_ptrace (struct task_struct *parent, struct task_struct *child) static int dummy_ptrace (struct task_struct *parent, struct task_struct *child)
{ {
...@@ -169,9 +171,30 @@ static void dummy_bprm_free_security (struct linux_binprm *bprm) ...@@ -169,9 +171,30 @@ static void dummy_bprm_free_security (struct linux_binprm *bprm)
return; return;
} }
static void dummy_bprm_compute_creds (struct linux_binprm *bprm) static inline int must_not_trace_exec (struct task_struct *p)
{ {
return; return ((p->ptrace & PT_PTRACED) && !(p->ptrace & PT_PTRACE_CAP))
|| atomic_read(&p->fs->count) > 1
|| atomic_read(&p->files->count) > 1
|| atomic_read(&p->sighand->count) > 1;
}
static void dummy_bprm_apply_creds (struct linux_binprm *bprm)
{
task_lock(current);
if (bprm->e_uid != current->uid || bprm->e_gid != current->gid) {
current->mm->dumpable = 0;
if (must_not_trace_exec(current) && !capable(CAP_SETUID)) {
bprm->e_uid = current->uid;
bprm->e_gid = current->gid;
}
}
current->suid = current->euid = current->fsuid = bprm->e_uid;
current->sgid = current->egid = current->fsgid = bprm->e_gid;
task_unlock(current);
} }
static int dummy_bprm_set_security (struct linux_binprm *bprm) static int dummy_bprm_set_security (struct linux_binprm *bprm)
...@@ -887,7 +910,7 @@ void security_fixup_ops (struct security_operations *ops) ...@@ -887,7 +910,7 @@ void security_fixup_ops (struct security_operations *ops)
set_to_dummy_if_null(ops, vm_enough_memory); set_to_dummy_if_null(ops, vm_enough_memory);
set_to_dummy_if_null(ops, bprm_alloc_security); set_to_dummy_if_null(ops, bprm_alloc_security);
set_to_dummy_if_null(ops, bprm_free_security); set_to_dummy_if_null(ops, bprm_free_security);
set_to_dummy_if_null(ops, bprm_compute_creds); set_to_dummy_if_null(ops, bprm_apply_creds);
set_to_dummy_if_null(ops, bprm_set_security); set_to_dummy_if_null(ops, bprm_set_security);
set_to_dummy_if_null(ops, bprm_check_security); set_to_dummy_if_null(ops, bprm_check_security);
set_to_dummy_if_null(ops, bprm_secureexec); set_to_dummy_if_null(ops, bprm_secureexec);
......
...@@ -90,7 +90,7 @@ static struct security_operations rootplug_security_ops = { ...@@ -90,7 +90,7 @@ static struct security_operations rootplug_security_ops = {
.capset_set = cap_capset_set, .capset_set = cap_capset_set,
.capable = cap_capable, .capable = cap_capable,
.bprm_compute_creds = cap_bprm_compute_creds, .bprm_apply_creds = cap_bprm_apply_creds,
.bprm_set_security = cap_bprm_set_security, .bprm_set_security = cap_bprm_set_security,
.task_post_setuid = cap_task_post_setuid, .task_post_setuid = cap_task_post_setuid,
......
...@@ -1745,7 +1745,7 @@ static inline void flush_unauthorized_files(struct files_struct * files) ...@@ -1745,7 +1745,7 @@ static inline void flush_unauthorized_files(struct files_struct * files)
spin_unlock(&files->file_lock); spin_unlock(&files->file_lock);
} }
static void selinux_bprm_compute_creds(struct linux_binprm *bprm) static void selinux_bprm_apply_creds(struct linux_binprm *bprm)
{ {
struct task_security_struct *tsec, *psec; struct task_security_struct *tsec, *psec;
struct bprm_security_struct *bsec; struct bprm_security_struct *bsec;
...@@ -1755,7 +1755,7 @@ static void selinux_bprm_compute_creds(struct linux_binprm *bprm) ...@@ -1755,7 +1755,7 @@ static void selinux_bprm_compute_creds(struct linux_binprm *bprm)
struct rlimit *rlim, *initrlim; struct rlimit *rlim, *initrlim;
int rc, i; int rc, i;
secondary_ops->bprm_compute_creds(bprm); secondary_ops->bprm_apply_creds(bprm);
tsec = current->security; tsec = current->security;
...@@ -2560,7 +2560,7 @@ static int selinux_task_setrlimit(unsigned int resource, struct rlimit *new_rlim ...@@ -2560,7 +2560,7 @@ static int selinux_task_setrlimit(unsigned int resource, struct rlimit *new_rlim
/* Control the ability to change the hard limit (whether /* Control the ability to change the hard limit (whether
lowering or raising it), so that the hard limit can lowering or raising it), so that the hard limit can
later be used as a safe reset point for the soft limit later be used as a safe reset point for the soft limit
upon context transitions. See selinux_bprm_compute_creds. */ upon context transitions. See selinux_bprm_apply_creds. */
if (old_rlim->rlim_max != new_rlim->rlim_max) if (old_rlim->rlim_max != new_rlim->rlim_max)
return task_has_perm(current, current, PROCESS__SETRLIMIT); return task_has_perm(current, current, PROCESS__SETRLIMIT);
...@@ -3971,7 +3971,7 @@ struct security_operations selinux_ops = { ...@@ -3971,7 +3971,7 @@ struct security_operations selinux_ops = {
.bprm_alloc_security = selinux_bprm_alloc_security, .bprm_alloc_security = selinux_bprm_alloc_security,
.bprm_free_security = selinux_bprm_free_security, .bprm_free_security = selinux_bprm_free_security,
.bprm_compute_creds = selinux_bprm_compute_creds, .bprm_apply_creds = selinux_bprm_apply_creds,
.bprm_set_security = selinux_bprm_set_security, .bprm_set_security = selinux_bprm_set_security,
.bprm_check_security = selinux_bprm_check_security, .bprm_check_security = selinux_bprm_check_security,
.bprm_secureexec = selinux_bprm_secureexec, .bprm_secureexec = selinux_bprm_secureexec,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment