From patchwork Wed Nov 9 03:46:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Seth Forshee X-Patchwork-Id: 692583 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3tDBvb49ywz9t1d; Wed, 9 Nov 2016 14:47:23 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical-com.20150623.gappssmtp.com header.i=@canonical-com.20150623.gappssmtp.com header.b="udcpMK+f"; dkim-atps=neutral Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1c4Jr0-00027c-9d; Wed, 09 Nov 2016 03:47:14 +0000 Received: from mail-it0-f51.google.com ([209.85.214.51]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1c4Jqv-000274-Kz for kernel-team@lists.ubuntu.com; Wed, 09 Nov 2016 03:47:09 +0000 Received: by mail-it0-f51.google.com with SMTP id q124so248576934itd.1 for ; Tue, 08 Nov 2016 19:47:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:in-reply-to:references; bh=GR+dATeJOnE06Ix+hDpGmdpkHQy6JEcE9vgZC9dm7LY=; b=udcpMK+fEnnCRXC0h5ylzOMvncv7mpp3uNdGAaV9aOUp/pPMmNZiZE5sEkkJ9dJsN8 6ooKgfQZEHtS+6zOJsReh44OHQepzWXEwgfbnD2MYLnN8NSuqTFC758oU/uQPAzJGrBP VxWZke7T0yJiyfnd2xsBGyNAexRqtMRvCxn9toQZE72S/dk8de3rlLmABwVCJGiqKO9F vhXr/W4qSeNjB/tGk0ZzGxA49jTUOXCT0vaXpBiX/KcngwtXmoyVYb+BqGuZ5c6Nzu4T UhPm4rXwsqJTMAu8OohAzaPXtK/RiQ3BSro7w8rHN2IhfPvmTq7ukPY5RoHZpj1mVK3/ KfLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=GR+dATeJOnE06Ix+hDpGmdpkHQy6JEcE9vgZC9dm7LY=; b=MK5unFnFDUbXEAbwTFJ5YxBBfBX/ZY79AETg0up9lPmvCLl4rpRILQT92Fxil7kjqX F9guYw/0XS858lQ81Po2OMtHVJnoE8IxzVbBDGF/8awDOWSBrm5KpdHOTfTLUBRtOdRy c2h3vI5f/EcRW6bcUKCMZoA/ZN/0ZWVhk7wMGF/zTa5tkMfH9GVOUrWhMla2oc+4T1g3 KElUwWwVHTh4xpTAKVi4WfIiGfQ2YG/3WiCus0Taku6eiOtQjyNO59XJrM6U8N9BG0VS HLG6fRQ9qA5B6RSRRFfSjVboSKFKNdEvwaRBKhdFDk2CEtAQw/4JUKPr9GhjKBateFO/ KJXQ== X-Gm-Message-State: ABUngvc6Lwep6kR2scIWR/jxw7xpMuR79SuaMDU/VI8CFXn0wBQuRBpjg0dKXs6p/qjaqJq1 X-Received: by 10.107.205.204 with SMTP id d195mr15860988iog.206.1478663228370; Tue, 08 Nov 2016 19:47:08 -0800 (PST) Received: from localhost ([2605:a601:aaf:a920:cd0b:269a:552f:c442]) by smtp.gmail.com with ESMTPSA id g127sm7405871itc.7.2016.11.08.19.47.07 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 08 Nov 2016 19:47:07 -0800 (PST) From: Seth Forshee To: kernel-team@lists.ubuntu.com Subject: [PATCH 2/2][trusty] UBUNTU: SAUCE: (no-up) mm: Add a user_ns owner to mm_struct and fix ptrace permission checks Date: Tue, 8 Nov 2016 21:46:59 -0600 Message-Id: <1478663225-108884-3-git-send-email-seth.forshee@canonical.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1478663225-108884-1-git-send-email-seth.forshee@canonical.com> References: <1478663225-108884-1-git-send-email-seth.forshee@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com From: "Eric W. Biederman" During exec dumpable is cleared if the file that is being executed is not readable by the user executing the file. A bug in ptrace_may_access allows reading the file if the executable happens to enter into a subordinate user namespace (aka clone(CLONE_NEWUSER), unshare(CLONE_NEWUSER), or setns(fd, CLONE_NEWUSER). This problem is fixed with only necessary userspace breakage by adding a user namespace owner to mm_struct, captured at the time of exec, so it is clear in which user namespace CAP_SYS_PTRACE must be present in to be able to safely give read permission to the executable. The function ptrace_may_access is modified to verify that the ptracer has CAP_SYS_ADMIN in task->mm->user_ns instead of task->cred->user_ns. This ensures that if the task changes it's cred into a subordinate user namespace it does not become ptraceable. The function ptrace_attach is modified to only set PT_PTRACE_CAP when CAP_SYS_PTRACE is held over task->mm->user_ns. The intent of PT_PTRACE_CAP is to be a flag to note that whatever permission changes the task might go through the tracer has sufficient permissions for it not to be an issue. task->cred->user_ns is always the same as or descendent of mm->user_ns. Which guarantees that having CAP_SYS_PTRACE over mm->user_ns is the worst case for the tasks credentials. To prevent regressions mm->dumpable and mm->user_ns are not considered when a task has no mm. As simply failing ptrace_may_attach causes regressions in privileged applications attempting to read things such as /proc//stat Cc: stable@vger.kernel.org Acked-by: Kees Cook Tested-by: Cyrill Gorcunov Fixes: 8409cca70561 ("userns: allow ptrace from non-init user namespaces") Signed-off-by: "Eric W. Biederman" (backported from commit 2e41414828bb0b066bde2f156cfa848c38531edf linux-next) CVE-2015-8709 Signed-off-by: Seth Forshee --- include/linux/mm_types.h | 1 + kernel/fork.c | 9 ++++++--- kernel/ptrace.c | 27 ++++++++++++--------------- mm/init-mm.c | 2 ++ 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index c21588ba9db9..16f159d2d0b2 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -420,6 +420,7 @@ struct mm_struct { */ struct task_struct __rcu *owner; #endif + struct user_namespace *user_ns; /* store ref to file /proc//exe symlink points to */ struct file *exe_file; diff --git a/kernel/fork.c b/kernel/fork.c index 937c7a1b9a35..34cba879ae4d 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -538,7 +538,8 @@ static void mm_init_aio(struct mm_struct *mm) #endif } -static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p) +static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p, + struct user_namespace *user_ns) { atomic_set(&mm->mm_users, 1); atomic_set(&mm->mm_count, 1); @@ -557,6 +558,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p) if (likely(!mm_alloc_pgd(mm))) { mm->def_flags = 0; mmu_notifier_mm_init(mm); + mm->user_ns = get_user_ns(user_ns); return mm; } @@ -594,7 +596,7 @@ struct mm_struct *mm_alloc(void) memset(mm, 0, sizeof(*mm)); mm_init_cpumask(mm); - return mm_init(mm, current); + return mm_init(mm, current, current_user_ns()); } /* @@ -609,6 +611,7 @@ void __mmdrop(struct mm_struct *mm) destroy_context(mm); mmu_notifier_mm_destroy(mm); check_mm(mm); + put_user_ns(mm->user_ns); free_mm(mm); } EXPORT_SYMBOL_GPL(__mmdrop); @@ -833,7 +836,7 @@ struct mm_struct *dup_mm(struct task_struct *tsk) #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && !USE_SPLIT_PMD_PTLOCKS mm->pmd_huge_pte = NULL; #endif - if (!mm_init(mm, tsk)) + if (!mm_init(mm, tsk, mm->user_ns)) goto fail_nomem; if (init_new_context(tsk, mm)) diff --git a/kernel/ptrace.c b/kernel/ptrace.c index be9760f8284a..9778967fa4f5 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -225,6 +225,7 @@ static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode) static int __ptrace_may_access(struct task_struct *task, unsigned int mode) { const struct cred *cred = current_cred(), *tcred; + struct mm_struct *mm; /* May we inspect the given task? * This check is used both for attaching with ptrace @@ -234,7 +235,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) * because setting up the necessary parent/child relationship * or halting the specified task is impossible. */ - int dumpable = 0; + /* Don't let security modules deny introspection */ if (same_thread_group(task, current)) return 0; @@ -253,16 +254,11 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) return -EPERM; ok: rcu_read_unlock(); - smp_rmb(); - if (task->mm) - dumpable = get_dumpable(task->mm); - rcu_read_lock(); - if (dumpable != SUID_DUMP_USER && - !ptrace_has_cap(__task_cred(task)->user_ns, mode)) { - rcu_read_unlock(); - return -EPERM; - } - rcu_read_unlock(); + mm = task->mm; + if (mm && + ((get_dumpable(mm) != SUID_DUMP_USER) && + !ptrace_has_cap(mm->user_ns, mode))) + return -EPERM; return security_ptrace_access_check(task, mode); } @@ -313,6 +309,11 @@ static int ptrace_attach(struct task_struct *task, long request, task_lock(task); retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH); + if (!retval) { + struct mm_struct *mm = task->mm; + if (mm && ns_capable(mm->user_ns, CAP_SYS_PTRACE)) + flags |= PT_PTRACE_CAP; + } task_unlock(task); if (retval) goto unlock_creds; @@ -326,10 +327,6 @@ static int ptrace_attach(struct task_struct *task, long request, if (seize) flags |= PT_SEIZED; - rcu_read_lock(); - if (ns_capable(__task_cred(task)->user_ns, CAP_SYS_PTRACE)) - flags |= PT_PTRACE_CAP; - rcu_read_unlock(); task->ptrace = flags; __ptrace_link(task, current); diff --git a/mm/init-mm.c b/mm/init-mm.c index a56a851908d2..975e49f00f34 100644 --- a/mm/init-mm.c +++ b/mm/init-mm.c @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -21,5 +22,6 @@ struct mm_struct init_mm = { .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), .mmlist = LIST_HEAD_INIT(init_mm.mmlist), + .user_ns = &init_user_ns, INIT_MM_CONTEXT(init_mm) };