Skip to content

Commit d049f74

Browse files
keestorvalds
authored andcommitted
exec/ptrace: fix get_dumpable() incorrect tests
The get_dumpable() return value is not boolean. Most users of the function actually want to be testing for non-SUID_DUMP_USER(1) rather than SUID_DUMP_DISABLE(0). The SUID_DUMP_ROOT(2) is also considered a protected state. Almost all places did this correctly, excepting the two places fixed in this patch. Wrong logic: if (dumpable == SUID_DUMP_DISABLE) { /* be protective */ } or if (dumpable == 0) { /* be protective */ } or if (!dumpable) { /* be protective */ } Correct logic: if (dumpable != SUID_DUMP_USER) { /* be protective */ } or if (dumpable != 1) { /* be protective */ } Without this patch, if the system had set the sysctl fs/suid_dumpable=2, a user was able to ptrace attach to processes that had dropped privileges to that user. (This may have been partially mitigated if Yama was enabled.) The macros have been moved into the file that declares get/set_dumpable(), which means things like the ia64 code can see them too. CVE-2013-2929 Reported-by: Vasily Kulikov <segoon@openwall.com> Signed-off-by: Kees Cook <keescook@chromium.org> Cc: "Luck, Tony" <tony.luck@intel.com> Cc: Oleg Nesterov <oleg@redhat.com> Cc: "Eric W. Biederman" <ebiederm@xmission.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 1c3fc3e commit d049f74

File tree

5 files changed

+13
-5
lines changed

5 files changed

+13
-5
lines changed

Diff for: arch/ia64/include/asm/processor.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ struct thread_struct {
319319
regs->loadrs = 0; \
320320
regs->r8 = get_dumpable(current->mm); /* set "don't zap registers" flag */ \
321321
regs->r12 = new_sp - 16; /* allocate 16 byte scratch area */ \
322-
if (unlikely(!get_dumpable(current->mm))) { \
322+
if (unlikely(get_dumpable(current->mm) != SUID_DUMP_USER)) { \
323323
/* \
324324
* Zap scratch regs to avoid leaking bits between processes with different \
325325
* uid/privileges. \

Diff for: fs/exec.c

+6
Original file line numberDiff line numberDiff line change
@@ -1669,6 +1669,12 @@ int __get_dumpable(unsigned long mm_flags)
16691669
return (ret > SUID_DUMP_USER) ? SUID_DUMP_ROOT : ret;
16701670
}
16711671

1672+
/*
1673+
* This returns the actual value of the suid_dumpable flag. For things
1674+
* that are using this for checking for privilege transitions, it must
1675+
* test against SUID_DUMP_USER rather than treating it as a boolean
1676+
* value.
1677+
*/
16721678
int get_dumpable(struct mm_struct *mm)
16731679
{
16741680
return __get_dumpable(mm->flags);

Diff for: include/linux/binfmts.h

-3
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,6 @@ extern void setup_new_exec(struct linux_binprm * bprm);
9999
extern void would_dump(struct linux_binprm *, struct file *);
100100

101101
extern int suid_dumpable;
102-
#define SUID_DUMP_DISABLE 0 /* No setuid dumping */
103-
#define SUID_DUMP_USER 1 /* Dump as user of process */
104-
#define SUID_DUMP_ROOT 2 /* Dump as root */
105102

106103
/* Stack area protections */
107104
#define EXSTACK_DEFAULT 0 /* Whatever the arch defaults to */

Diff for: include/linux/sched.h

+4
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,10 @@ static inline void arch_pick_mmap_layout(struct mm_struct *mm) {}
323323
extern void set_dumpable(struct mm_struct *mm, int value);
324324
extern int get_dumpable(struct mm_struct *mm);
325325

326+
#define SUID_DUMP_DISABLE 0 /* No setuid dumping */
327+
#define SUID_DUMP_USER 1 /* Dump as user of process */
328+
#define SUID_DUMP_ROOT 2 /* Dump as root */
329+
326330
/* mm flags */
327331
/* dumpable bits */
328332
#define MMF_DUMPABLE 0 /* core dump is permitted */

Diff for: kernel/ptrace.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,8 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
257257
if (task->mm)
258258
dumpable = get_dumpable(task->mm);
259259
rcu_read_lock();
260-
if (!dumpable && !ptrace_has_cap(__task_cred(task)->user_ns, mode)) {
260+
if (dumpable != SUID_DUMP_USER &&
261+
!ptrace_has_cap(__task_cred(task)->user_ns, mode)) {
261262
rcu_read_unlock();
262263
return -EPERM;
263264
}

0 commit comments

Comments
 (0)