Messages in this thread | | | Date | Tue, 18 Oct 2016 15:57:41 +0200 | From | Jann Horn <> | Subject | Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access |
| |
On Tue, Oct 18, 2016 at 03:50:32PM +0200, Michal Hocko wrote: > On Mon 17-10-16 11:39:49, Eric W. Biederman 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. > > I haven't studied your patch too deeply but one thing that immediately > raised a red flag was that mm might be shared between processes (aka > thread groups).
You're conflating things. Threads always share memory, but sharing memory doesn't imply being part of the same thread group.
> What prevents those two to sit in different user > namespaces?
For thread groups: You can't change user namespace in a thread group with more than one task.
For shared mm: Yeah, I think that could happen - but it doesn't matter. The patch just needs the mm to determine the namespace in which the mm was created, and that's always the same for tasks that share mm. [unhandled content-type:application/pgp-signature] | |