Commit bfedb589 authored by Eric W. Biederman's avatar Eric W. Biederman

mm: Add a user_ns owner to mm_struct and fix ptrace permission checks

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/<pid>/stat

Cc: stable@vger.kernel.org
Acked-by: default avatarKees Cook <keescook@chromium.org>
Tested-by: default avatarCyrill Gorcunov <gorcunov@openvz.org>
Fixes: 8409cca7 ("userns: allow ptrace from non-init user namespaces")
Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
parent 9c763584
...@@ -473,6 +473,7 @@ struct mm_struct { ...@@ -473,6 +473,7 @@ struct mm_struct {
*/ */
struct task_struct __rcu *owner; struct task_struct __rcu *owner;
#endif #endif
struct user_namespace *user_ns;
/* store ref to file /proc/<pid>/exe symlink points to */ /* store ref to file /proc/<pid>/exe symlink points to */
struct file __rcu *exe_file; struct file __rcu *exe_file;
......
...@@ -745,7 +745,8 @@ static void mm_init_owner(struct mm_struct *mm, struct task_struct *p) ...@@ -745,7 +745,8 @@ static void mm_init_owner(struct mm_struct *mm, struct task_struct *p)
#endif #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)
{ {
mm->mmap = NULL; mm->mmap = NULL;
mm->mm_rb = RB_ROOT; mm->mm_rb = RB_ROOT;
...@@ -785,6 +786,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p) ...@@ -785,6 +786,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p)
if (init_new_context(p, mm)) if (init_new_context(p, mm))
goto fail_nocontext; goto fail_nocontext;
mm->user_ns = get_user_ns(user_ns);
return mm; return mm;
fail_nocontext: fail_nocontext:
...@@ -830,7 +832,7 @@ struct mm_struct *mm_alloc(void) ...@@ -830,7 +832,7 @@ struct mm_struct *mm_alloc(void)
return NULL; return NULL;
memset(mm, 0, sizeof(*mm)); memset(mm, 0, sizeof(*mm));
return mm_init(mm, current); return mm_init(mm, current, current_user_ns());
} }
/* /*
...@@ -845,6 +847,7 @@ void __mmdrop(struct mm_struct *mm) ...@@ -845,6 +847,7 @@ void __mmdrop(struct mm_struct *mm)
destroy_context(mm); destroy_context(mm);
mmu_notifier_mm_destroy(mm); mmu_notifier_mm_destroy(mm);
check_mm(mm); check_mm(mm);
put_user_ns(mm->user_ns);
free_mm(mm); free_mm(mm);
} }
EXPORT_SYMBOL_GPL(__mmdrop); EXPORT_SYMBOL_GPL(__mmdrop);
...@@ -1126,7 +1129,7 @@ static struct mm_struct *dup_mm(struct task_struct *tsk) ...@@ -1126,7 +1129,7 @@ static struct mm_struct *dup_mm(struct task_struct *tsk)
memcpy(mm, oldmm, sizeof(*mm)); memcpy(mm, oldmm, sizeof(*mm));
if (!mm_init(mm, tsk)) if (!mm_init(mm, tsk, mm->user_ns))
goto fail_nomem; goto fail_nomem;
err = dup_mmap(mm, oldmm); err = dup_mmap(mm, oldmm);
......
...@@ -220,7 +220,7 @@ static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode) ...@@ -220,7 +220,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) static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
{ {
const struct cred *cred = current_cred(), *tcred; const struct cred *cred = current_cred(), *tcred;
int dumpable = 0; struct mm_struct *mm;
kuid_t caller_uid; kuid_t caller_uid;
kgid_t caller_gid; kgid_t caller_gid;
...@@ -271,16 +271,11 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) ...@@ -271,16 +271,11 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
return -EPERM; return -EPERM;
ok: ok:
rcu_read_unlock(); rcu_read_unlock();
smp_rmb(); mm = task->mm;
if (task->mm) if (mm &&
dumpable = get_dumpable(task->mm); ((get_dumpable(mm) != SUID_DUMP_USER) &&
rcu_read_lock(); !ptrace_has_cap(mm->user_ns, mode)))
if (dumpable != SUID_DUMP_USER &&
!ptrace_has_cap(__task_cred(task)->user_ns, mode)) {
rcu_read_unlock();
return -EPERM; return -EPERM;
}
rcu_read_unlock();
return security_ptrace_access_check(task, mode); return security_ptrace_access_check(task, mode);
} }
...@@ -331,6 +326,11 @@ static int ptrace_attach(struct task_struct *task, long request, ...@@ -331,6 +326,11 @@ static int ptrace_attach(struct task_struct *task, long request,
task_lock(task); task_lock(task);
retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS); retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
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); task_unlock(task);
if (retval) if (retval)
goto unlock_creds; goto unlock_creds;
...@@ -344,10 +344,6 @@ static int ptrace_attach(struct task_struct *task, long request, ...@@ -344,10 +344,6 @@ static int ptrace_attach(struct task_struct *task, long request,
if (seize) if (seize)
flags |= PT_SEIZED; 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; task->ptrace = flags;
__ptrace_link(task, current); __ptrace_link(task, current);
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <linux/cpumask.h> #include <linux/cpumask.h>
#include <linux/atomic.h> #include <linux/atomic.h>
#include <linux/user_namespace.h>
#include <asm/pgtable.h> #include <asm/pgtable.h>
#include <asm/mmu.h> #include <asm/mmu.h>
...@@ -21,5 +22,6 @@ struct mm_struct init_mm = { ...@@ -21,5 +22,6 @@ struct mm_struct init_mm = {
.mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem),
.page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock),
.mmlist = LIST_HEAD_INIT(init_mm.mmlist), .mmlist = LIST_HEAD_INIT(init_mm.mmlist),
.user_ns = &init_user_ns,
INIT_MM_CONTEXT(init_mm) INIT_MM_CONTEXT(init_mm)
}; };
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