exec/ptrace: fix get_dumpable() incorrect tests

commit d049f74f2dbe71354d43d393ac3a188947811348 upstream

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: Tony Luck <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>
[dannf: backported to Debian's 2.6.32]
Signed-off-by: Willy Tarreau <w@1wt.eu>

diff --git a/arch/ia64/include/asm/processor.h b/arch/ia64/include/asm/processor.h
index 3eaeedf1aef2cf20d5a8c707d64ddd34746c18b2..d77b3425cc9271592e50263fc0c90616d54fdbf3 100644 (file)
--- a/arch/ia64/include/asm/processor.h
+++ b/arch/ia64/include/asm/processor.h
@@ -361,7 +361,7 @@ struct thread_struct {
        regs->loadrs = 0;                                                                       \
        regs->r8 = get_dumpable(current->mm);   /* set "don't zap registers" flag */            \
        regs->r12 = new_sp - 16;        /* allocate 16 byte scratch area */                     \
-       if (unlikely(!get_dumpable(current->mm))) {                                                     \
+       if (unlikely(get_dumpable(current->mm) != SUID_DUMP_USER)) {    \
                /*                                                                              \
                 * Zap scratch regs to avoid leaking bits between processes with different      \
                 * uid/privileges.                                                              \

diff --git a/fs/exec.c b/fs/exec.c
index feb243522284e66ad4521de6a6444cad319d0fd0..c32ae3444622a8185288c674fed3c17313117372 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1793,6 +1793,12 @@ void set_dumpable(struct mm_struct *mm, int value)
        }
 }
 
+/*
+ * This returns the actual value of the suid_dumpable flag. For things
+ * that are using this for checking for privilege transitions, it must
+ * test against SUID_DUMP_USER rather than treating it as a boolean
+ * value.
+ */
 int get_dumpable(struct mm_struct *mm)
 {
        int ret;

diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 9ffffec7ba082d34aed22d7505364b0edb104537..8eab6288ddf1db8dae1c92460aeb9772a56b77cb 100644 (file)
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -107,9 +107,6 @@ extern int flush_old_exec(struct linux_binprm * bprm);
 extern void setup_new_exec(struct linux_binprm * bprm);
 
 extern int suid_dumpable;
-#define SUID_DUMP_DISABLE      0       /* No setuid dumping */
-#define SUID_DUMP_USER         1       /* Dump as user of process */
-#define SUID_DUMP_ROOT         2       /* Dump as root */
 
 /* Stack area protections */
 #define EXSTACK_DEFAULT   0    /* Whatever the arch defaults to */

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 73c3b9ba95dc4ad86997c98b763fb415f2259143..56e1771eb6c8f1dbbc33d624756b6658e3b53461 100644 (file)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -442,6 +442,10 @@ static inline unsigned long get_mm_hiwater_vm(struct mm_struct *mm)
 extern void set_dumpable(struct mm_struct *mm, int value);
 extern int get_dumpable(struct mm_struct *mm);
 
+#define SUID_DUMP_DISABLE      0       /* No setuid dumping */
+#define SUID_DUMP_USER         1       /* Dump as user of process */
+#define SUID_DUMP_ROOT         2       /* Dump as root */
+
 /* mm flags */
 /* dumpable bits */
 #define MMF_DUMPABLE      0  /* core dump is permitted */

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index d9c8c47ec994d945ec01775a335570ec1d356448..4185220acbefe20d490870ae633e33f4b912235d 100644 (file)
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -187,7 +187,7 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
        smp_rmb();
        if (task->mm)
                dumpable = get_dumpable(task->mm);
-       if (!dumpable && !capable(CAP_SYS_PTRACE))
+       if (dumpable != SUID_DUMP_USER && !capable(CAP_SYS_PTRACE))
                return -EPERM;
 
        return security_ptrace_access_check(task, mode);