Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1503013

Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access

From Jann Horn <jann@thejh.net>
Newsgroups linux.kernel
Subject Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access
Date 2016-10-18 16:00 +0200
Message-ID <stD5U-2pf-23@gated-at.bofh.it> (permalink)
References <stjTA-6sY-53@gated-at.bofh.it> <stD5T-2pf-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

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.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Michal Hocko <mhocko@kernel.org> - 2016-10-18 16:00 +0200
  Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Jann Horn <jann@thejh.net> - 2016-10-18 16:00 +0200
  Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access ebiederm@xmission.com (Eric W. Biederman) - 2016-10-18 17:00 +0200
    Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Jann Horn <jann@thejh.net> - 2016-10-18 17:10 +0200
      Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access ebiederm@xmission.com (Eric W. Biederman) - 2016-10-18 17:40 +0200
        Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Jann Horn <jann@thejh.net> - 2016-10-18 21:20 +0200
          Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access ebiederm@xmission.com (Eric W. Biederman) - 2016-10-18 23:10 +0200
            [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-18 23:20 +0200
              Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Amir Goldstein <amir73il@gmail.com> - 2016-10-19 08:20 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 17:10 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 19:10 +0200
              Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Andy Lutomirski <luto@amacapital.net> - 2016-10-19 17:40 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 19:00 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not  read. Jann Horn <jann@thejh.net> - 2016-10-19 19:30 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Andy Lutomirski <luto@amacapital.net> - 2016-10-19 19:40 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 20:00 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Andy Lutomirski <luto@amacapital.net> - 2016-10-19 20:40 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. ebiederm@xmission.com (Eric W. Biederman) - 2016-10-19 23:30 +0200
                Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Andy Lutomirski <luto@amacapital.net> - 2016-10-20 01:20 +0200
    Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix  ptrace_may_access Michal Hocko <mhocko@kernel.org> - 2016-10-18 20:10 +0200

csiph-web