ptrace: slightly saner 'get_dumpable()' logic

commit 31e62c2ebbfdc3fe3dbdf5e02c92a9dc67087a3a upstream.

The 'dumpability' of a task is fundamentally about the memory image of
the task - the concept comes from whether it can core dump or not - and
makes no sense when you don't have an associated mm.

And almost all users do in fact use it only for the case where the task
has a mm pointer.

But we have one odd special case: ptrace_may_access() uses 'dumpable' to
check various other things entirely independently of the MM (typically
explicitly using flags like PTRACE_MODE_READ_FSCREDS).  Including for
threads that no longer have a VM (and maybe never did, like most kernel
threads).

It's not what this flag was designed for, but it is what it is.

The ptrace code does check that the uid/gid matches, so you do have to
be uid-0 to see kernel thread details, but this means that the
traditional "drop capabilities" model doesn't make any difference for
this all.

Make it all make a *bit* more sense by saying that if you don't have a
MM pointer, we'll use a cached "last dumpability" flag if the thread
ever had a MM (it will be zero for kernel threads since it is never
set), and require a proper CAP_SYS_PTRACE capability to override.

Reported-by: Qualys Security Advisory <qsa@qualys.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Kees Cook <kees@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 37ee1b1c9ed5f3dc43b255d2f3010f4c595b31e3..3efada5573ea0a95b86afff9d5f91a256ea1d9fa 100644 (file)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -910,6 +910,9 @@ struct task_struct {
         */
        unsigned                        sched_remote_wakeup:1;
 
+       /* Save user-dumpable when mm goes away */
+       unsigned                        user_dumpable:1;
+
        /* Bit to tell LSMs we're in execve(): */
        unsigned                        in_execve:1;
        unsigned                        in_iowait:1;

diff --git a/kernel/exit.c b/kernel/exit.c
index 10af26a4ab010b11439cdc16c27783e501c8fb5a..12a5332b8867777ae56fcaadc472ea32d9fe4db0 100644 (file)
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -555,6 +555,7 @@ static void exit_mm(void)
         */
        smp_mb__after_spinlock();
        local_irq_disable();
+       current->user_dumpable = (get_dumpable(mm) == SUID_DUMP_USER);
        current->mm = NULL;
        membarrier_update_current_mm(NULL);
        enter_lazy_tlb(mm, current);

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 54482193e1edc2c665a8f32f08c4d81300757dbc..435848809ebcd6f40bc742404f4b12832ee7e64a 100644 (file)
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -282,11 +282,24 @@ static bool ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
        return ns_capable(ns, CAP_SYS_PTRACE);
 }
 
+static bool task_still_dumpable(struct task_struct *task, unsigned int mode)
+{
+       struct mm_struct *mm = task->mm;
+       if (mm) {
+               if (get_dumpable(mm) == SUID_DUMP_USER)
+                       return true;
+               return ptrace_has_cap(mm->user_ns, mode);
+       }
+
+       if (task->user_dumpable)
+               return true;
+       return ptrace_has_cap(&init_user_ns, mode);
+}
+
 /* Returns 0 on success, -errno on denial. */
 static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 {
        const struct cred *cred = current_cred(), *tcred;
-       struct mm_struct *mm;
        kuid_t caller_uid;
        kgid_t caller_gid;
 
@@ -347,11 +360,8 @@ ok:
         * Pairs with a write barrier in commit_creds().
         */
        smp_rmb();
-       mm = task->mm;
-       if (mm &&
-           ((get_dumpable(mm) != SUID_DUMP_USER) &&
-            !ptrace_has_cap(mm->user_ns, mode)))
-           return -EPERM;
+       if (!task_still_dumpable(task, mode))
+               return -EPERM;
 
        return security_ptrace_access_check(task, mode);
 }