All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jann Horn <jann-XZ1E9jl8jIdeoWH0uzbU5w@public.gmane.org>
To: Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Containers
	<containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	"Eric W. Biederman"
	<ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access
Date: Tue, 18 Oct 2016 15:57:41 +0200	[thread overview]
Message-ID: <20161018135741.GO14666@pc.thejh.net> (raw)
In-Reply-To: <20161018135031.GB13117-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1721 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 205 bytes --]

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

WARNING: multiple messages have this Message-ID (diff)
From: Jann Horn <jann@thejh.net>
To: Michal Hocko <mhocko@kernel.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	linux-kernel@vger.kernel.org,
	Linux Containers <containers@lists.linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Andy Lutomirski <luto@amacapital.net>,
	linux-mm@kvack.org, linux-fsdevel@vger.kernel.org
Subject: Re: [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access
Date: Tue, 18 Oct 2016 15:57:41 +0200	[thread overview]
Message-ID: <20161018135741.GO14666@pc.thejh.net> (raw)
In-Reply-To: <20161018135031.GB13117@dhcp22.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 1721 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2016-10-18 13:57 UTC|newest]

Thread overview: 159+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-17 16:39 [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access Eric W. Biederman
2016-10-17 16:39 ` Eric W. Biederman
2016-10-17 16:39 ` Eric W. Biederman
     [not found] ` <87twcbq696.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
2016-10-17 17:25   ` Jann Horn
2016-10-18 13:50   ` Michal Hocko
2016-10-17 17:25 ` Jann Horn
     [not found]   ` <20161017172547.GJ14666-J1fxOzX/cBvk1uMJSBkQmQ@public.gmane.org>
2016-10-17 17:33     ` Eric W. Biederman
2016-10-17 17:33       ` Eric W. Biederman
2016-10-17 17:33       ` Eric W. Biederman
2016-10-18 13:50 ` Michal Hocko
2016-10-18 13:50   ` Michal Hocko
     [not found]   ` <20161018135031.GB13117-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2016-10-18 13:57     ` Jann Horn [this message]
2016-10-18 13:57       ` Jann Horn
2016-10-18 14:56     ` Eric W. Biederman
2016-10-18 14:56       ` Eric W. Biederman
2016-10-18 14:56       ` Eric W. Biederman
     [not found]       ` <8737jt903u.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-10-18 15:05         ` Jann Horn
2016-10-18 18:06         ` Michal Hocko
2016-10-18 15:05       ` Jann Horn
     [not found]         ` <20161018150507.GP14666-J1fxOzX/cBvk1uMJSBkQmQ@public.gmane.org>
2016-10-18 15:35           ` Eric W. Biederman
2016-10-18 15:35             ` Eric W. Biederman
2016-10-18 15:35             ` Eric W. Biederman
     [not found]             ` <87twc9656s.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-10-18 19:12               ` Jann Horn
2016-10-18 19:12                 ` Jann Horn
2016-10-18 19:12                 ` Jann Horn
2016-10-18 21:07                 ` Eric W. Biederman
2016-10-18 21:07                   ` Eric W. Biederman
2016-10-18 21:15                   ` [REVIEW][PATCH] exec: Don't exec files the userns root can not read Eric W. Biederman
2016-10-18 21:15                     ` Eric W. Biederman
     [not found]                     ` <87k2d5nytz.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-10-19  6:13                       ` Amir Goldstein
2016-10-19 15:30                       ` Andy Lutomirski
2016-10-19  6:13                     ` Amir Goldstein
2016-10-19  6:13                       ` Amir Goldstein
     [not found]                       ` <CAOQ4uxjyZF346vq-Oi=HwB=jj6ePycHBnEfvVPet9KqPxL9mgg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-19 13:33                         ` Eric W. Biederman
2016-10-19 13:33                       ` Eric W. Biederman
2016-10-19 13:33                         ` Eric W. Biederman
     [not found]                         ` <87mvi0mpix.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-10-19 17:04                           ` Eric W. Biederman
2016-10-19 17:04                             ` Eric W. Biederman
2016-10-19 17:04                             ` Eric W. Biederman
2016-10-19 15:30                     ` Andy Lutomirski
2016-10-19 15:30                       ` Andy Lutomirski
     [not found]                       ` <CALCETrU4SZYUEPrv4JkpUpA+0sZ=EirZRftRDp+a5hce5E7HgA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-19 16:52                         ` Eric W. Biederman
2016-10-19 16:52                       ` Eric W. Biederman
2016-10-19 16:52                         ` Eric W. Biederman
2016-10-19 16:52                         ` Eric W. Biederman
2016-10-19 17:29                         ` Jann Horn
2016-10-19 17:29                           ` Jann Horn
2016-10-19 17:32                           ` Andy Lutomirski
2016-10-19 17:32                             ` Andy Lutomirski
     [not found]                             ` <CALCETrWSY1SRse5oqSwZ=goQ+ZALd2XcTP3SZ8ry49C8rNd98Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-19 17:55                               ` Eric W. Biederman
2016-10-19 17:55                             ` Eric W. Biederman
2016-10-19 17:55                               ` Eric W. Biederman
2016-10-19 17:55                               ` Eric W. Biederman
     [not found]                               ` <87pomwi5p2.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-10-19 18:38                                 ` Andy Lutomirski
2016-10-19 18:38                                   ` Andy Lutomirski
2016-10-19 18:38                                   ` Andy Lutomirski
     [not found]                                   ` <CALCETrUz2oU6OYwQ9K4M-SUg6FeDsd6Q1gf1w-cJRGg2PdmK8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-10-19 21:26                                     ` Eric W. Biederman
2016-10-19 21:26                                   ` Eric W. Biederman
2016-10-19 21:26                                     ` Eric W. Biederman
2016-10-19 21:26                                     ` Eric W. Biederman
     [not found]                                     ` <87pomwghda.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-10-19 23:17                                       ` Andy Lutomirski
2016-10-19 23:17                                     ` Andy Lutomirski
2016-10-19 23:17                                       ` Andy Lutomirski
2016-11-17 17:02                                       ` [REVIEW][PATCH 0/3] Fixing ptrace vs exec vs userns interactions Eric W. Biederman
2016-11-17 17:02                                         ` Eric W. Biederman
2016-11-17 17:02                                         ` Eric W. Biederman
2016-11-17 17:05                                         ` [REVIEW][PATCH 1/3] ptrace: Capture the ptracer's creds not PT_PTRACE_CAP Eric W. Biederman
2016-11-17 17:05                                           ` Eric W. Biederman
2016-11-17 17:05                                           ` Eric W. Biederman
     [not found]                                           ` <87oa1eavfx.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-11-17 23:14                                             ` Kees Cook
2016-11-17 23:27                                             ` Andy Lutomirski
2016-11-17 23:14                                           ` Kees Cook
2016-11-17 23:14                                             ` Kees Cook
     [not found]                                             ` <CAGXu5jKbVkCGVSoxNQ=pTCBX1Boe3rPR1P56P-kR9AHWYHBs2w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-18 18:56                                               ` Eric W. Biederman
2016-11-18 18:56                                             ` Eric W. Biederman
2016-11-18 18:56                                               ` Eric W. Biederman
2016-11-18 18:56                                               ` Eric W. Biederman
2016-11-17 23:27                                           ` Andy Lutomirski
2016-11-17 23:27                                             ` Andy Lutomirski
     [not found]                                             ` <CALCETrUSnPfzpabQMNuyOu09j9QDzRDeoQVF_U51=ow3bP5pkw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-17 23:44                                               ` Eric W. Biederman
2016-11-17 23:44                                             ` Eric W. Biederman
2016-11-17 23:44                                               ` Eric W. Biederman
2016-11-17 23:44                                               ` Eric W. Biederman
2016-11-17 17:08                                         ` [REVIEW][PATCH 2/3] exec: Don't allow ptracing an exec of an unreadable file Eric W. Biederman
2016-11-17 17:08                                           ` Eric W. Biederman
2016-11-17 17:08                                           ` Eric W. Biederman
2016-11-17 20:47                                           ` Willy Tarreau
2016-11-17 20:47                                             ` Willy Tarreau
     [not found]                                             ` <20161117204707.GB10421-K+wRfnb2/UA@public.gmane.org>
2016-11-17 21:07                                               ` Kees Cook
2016-11-17 21:07                                             ` Kees Cook
2016-11-17 21:07                                               ` Kees Cook
2016-11-17 21:32                                               ` Willy Tarreau
2016-11-17 21:32                                                 ` Willy Tarreau
     [not found]                                                 ` <20161117213258.GA10839-K+wRfnb2/UA@public.gmane.org>
2016-11-17 21:51                                                   ` Eric W. Biederman
2016-11-17 21:51                                                 ` Eric W. Biederman
2016-11-17 21:51                                                   ` Eric W. Biederman
2016-11-17 21:51                                                   ` Eric W. Biederman
     [not found]                                                   ` <874m3522sy.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-11-17 22:50                                                     ` [REVIEW][PATCH 2/3] ptrace: Don't allow accessing an undumpable mm Eric W. Biederman
2016-11-17 22:50                                                   ` Eric W. Biederman
2016-11-17 22:50                                                     ` Eric W. Biederman
2016-11-17 22:50                                                     ` Eric W. Biederman
     [not found]                                                     ` <87shqpzpok.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-11-17 23:17                                                       ` Kees Cook
2016-11-17 23:17                                                     ` Kees Cook
2016-11-17 23:17                                                       ` Kees Cook
     [not found]                                               ` <CAGXu5jJc6TmzdVp+4OMDAt5Kd68hHbNBXaRPD8X0+m558hx3qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-17 21:32                                                 ` [REVIEW][PATCH 2/3] exec: Don't allow ptracing an exec of an unreadable file Willy Tarreau
2016-11-17 23:28                                                 ` Andy Lutomirski
2016-11-17 23:28                                                   ` Andy Lutomirski
2016-11-17 23:28                                                   ` Andy Lutomirski
2016-11-17 23:29                                           ` Andy Lutomirski
2016-11-17 23:29                                             ` Andy Lutomirski
     [not found]                                             ` <CALCETrUvKpRCXRE+K512E_q9-o8Gzgb+3XsAzSo+ZFdgqeX-eQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-17 23:55                                               ` Eric W. Biederman
2016-11-17 23:55                                             ` Eric W. Biederman
2016-11-17 23:55                                               ` Eric W. Biederman
2016-11-17 23:55                                               ` Eric W. Biederman
2016-11-18  0:10                                               ` Andy Lutomirski
2016-11-18  0:10                                                 ` Andy Lutomirski
2016-11-18  0:35                                                 ` Eric W. Biederman
2016-11-18  0:35                                                   ` Eric W. Biederman
2016-11-18  0:35                                                   ` Eric W. Biederman
     [not found]                                                 ` <CALCETrX=61Sk9qim+Psjn83gohuizEsrpUC9gF-vwQTtR4GuJw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-18  0:35                                                   ` Eric W. Biederman
     [not found]                                               ` <87mvgxwtjv.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-11-18  0:10                                                 ` Andy Lutomirski
     [not found]                                           ` <87inrmavax.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-11-17 20:47                                             ` Willy Tarreau
2016-11-17 23:29                                             ` Andy Lutomirski
2016-11-17 17:10                                         ` [REVIEW][PATCH 3/3] exec: Ensure mm->user_ns contains the execed files Eric W. Biederman
2016-11-17 17:10                                           ` Eric W. Biederman
2016-11-17 17:10                                           ` Eric W. Biederman
     [not found]                                         ` <87twb6avk8.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-11-17 17:05                                           ` [REVIEW][PATCH 1/3] ptrace: Capture the ptracer's creds not PT_PTRACE_CAP Eric W. Biederman
2016-11-17 17:08                                           ` [REVIEW][PATCH 2/3] exec: Don't allow ptracing an exec of an unreadable file Eric W. Biederman
2016-11-17 17:10                                           ` [REVIEW][PATCH 3/3] exec: Ensure mm->user_ns contains the execed files Eric W. Biederman
2016-11-19  7:17                                           ` [REVIEW][PATCH 0/3] Fixing ptrace vs exec vs userns interactions Willy Tarreau
2016-11-19  7:17                                         ` Willy Tarreau
2016-11-19  7:17                                           ` Willy Tarreau
2016-11-19  9:28                                           ` Willy Tarreau
2016-11-19  9:28                                             ` Willy Tarreau
2016-11-19  9:33                                             ` Willy Tarreau
2016-11-19  9:33                                               ` Willy Tarreau
     [not found]                                             ` <20161119092804.GA13553-K+wRfnb2/UA@public.gmane.org>
2016-11-19  9:33                                               ` Willy Tarreau
2016-11-19 18:44                                               ` Eric W. Biederman
2016-11-19 18:44                                             ` Eric W. Biederman
2016-11-19 18:44                                               ` Eric W. Biederman
2016-11-19 18:44                                               ` Eric W. Biederman
     [not found]                                           ` <20161119071700.GA13347-K+wRfnb2/UA@public.gmane.org>
2016-11-19  9:28                                             ` Willy Tarreau
2016-11-19 18:35                                             ` Eric W. Biederman
2016-11-19 18:35                                           ` Eric W. Biederman
2016-11-19 18:35                                             ` Eric W. Biederman
2016-11-19 18:35                                             ` Eric W. Biederman
2016-11-19 18:37                                             ` Eric W. Biederman
2016-11-19 18:37                                               ` Eric W. Biederman
2016-11-19 18:37                                               ` Eric W. Biederman
     [not found]                                             ` <87d1hrjp23.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-11-19 18:37                                               ` Eric W. Biederman
     [not found]                                       ` <CALCETrXA2EnE8X3HzetLG6zS8YSVjJQJrsSumTfvEcGq=r5vsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-17 17:02                                         ` Eric W. Biederman
     [not found]                           ` <20161019172917.GE1210-GiL72Q0nGm9Crx9znvW9yA@public.gmane.org>
2016-10-19 17:32                             ` [REVIEW][PATCH] exec: Don't exec files the userns root can not read Andy Lutomirski
     [not found]                         ` <87y41kjn6l.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-10-19 17:29                           ` Jann Horn
2016-10-19 18:36                           ` Andy Lutomirski
2016-10-19 18:36                         ` Andy Lutomirski
     [not found]                   ` <87r37dnz74.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2016-10-18 21:15                     ` Eric W. Biederman
     [not found]                 ` <20161018191206.GA1210-GiL72Q0nGm9Crx9znvW9yA@public.gmane.org>
2016-10-18 21:07                   ` [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access Eric W. Biederman
2016-10-18 18:06       ` Michal Hocko
2016-10-18 18:06         ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161018135741.GO14666@pc.thejh.net \
    --to=jann-xz1e9jl8jideowh0uzbu5w@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org \
    --cc=luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org \
    --cc=mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.