Re: [REVIEW][PATCH v2] mm: Add a user_ns owner to mm_struct and fix ptrace permission checks

[Kees Cook]

On Thu, Oct 27, 2016 at 8:54 AM, Eric W. Biederman
<ebiederm@xxxxxxxxxxxx> wrote:
>
> 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.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 8409cca70561 ("userns: allow ptrace from non-init user namespaces")
> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>

Thanks!

Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>

-Kees

-- 
Kees Cook
Nexus Security