[PATCH] selinux: Allow context mounts for unpriviliged overlayfs

[PATCH] selinux: Allow context mounts for unpriviliged overlayfs

[Vivek Goyal]

Now overlayfs allow unpriviliged mounts. That is root inside a non-init
user namespace can mount overlayfs. This was added in 5.10 kernel.

Giuseppe tried to mount overlayfs with option "context" and it failed
with error -EACCESS.

$ su test
$ unshare -rm
$ mkdir -p lower upper work merged
$ mount -t overlay -o lowerdir=lower,workdir=work,upperdir=upper,userxattr,context='system_u:object_r:container_file_t:s0' none merged

This fails with -EACCESS. It works if option "-o context" is not specified.

Little debugging showed that selinux_set_mnt_opts() returns -EACCESS.

So this patch adds "overlay" to the list, where it is fine to specific
context from non init_user_ns.

Reported-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 security/selinux/hooks.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: redhat-linux/security/selinux/hooks.c
===================================================================
--- redhat-linux.orig/security/selinux/hooks.c	2021-02-09 10:56:12.954988476 -0500
+++ redhat-linux/security/selinux/hooks.c	2021-02-09 14:36:33.136205330 -0500
@@ -733,7 +733,8 @@ static int selinux_set_mnt_opts(struct s
 	if (sb->s_user_ns != &init_user_ns &&
 	    strcmp(sb->s_type->name, "tmpfs") &&
 	    strcmp(sb->s_type->name, "ramfs") &&
-	    strcmp(sb->s_type->name, "devpts")) {
+	    strcmp(sb->s_type->name, "devpts") &&
+	    strcmp(sb->s_type->name, "overlay")) {
 		if (context_sid || fscontext_sid || rootcontext_sid ||
 		    defcontext_sid) {
 			rc = -EACCES;

Re: [PATCH] selinux: Allow context mounts for unpriviliged overlayfs

[Paul Moore]

On Tue, Feb 9, 2021 at 3:02 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> Now overlayfs allow unpriviliged mounts. That is root inside a non-init
> user namespace can mount overlayfs. This was added in 5.10 kernel.
>
> Giuseppe tried to mount overlayfs with option "context" and it failed
> with error -EACCESS.
>
> $ su test
> $ unshare -rm
> $ mkdir -p lower upper work merged
> $ mount -t overlay -o lowerdir=lower,workdir=work,upperdir=upper,userxattr,context='system_u:object_r:container_file_t:s0' none merged
>
> This fails with -EACCESS. It works if option "-o context" is not specified.
>
> Little debugging showed that selinux_set_mnt_opts() returns -EACCESS.
>
> So this patch adds "overlay" to the list, where it is fine to specific
> context from non init_user_ns.
>
> Reported-by: Giuseppe Scrivano <gscrivan@redhat.com>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  security/selinux/hooks.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

This seems reasonable, but since we are at -rc7 this week it will need
to wait until after the upcoming merge window.  It's too late in the
cycle for new features.

-- 
paul moore
www.paul-moore.com

Re: [PATCH] selinux: Allow context mounts for unpriviliged overlayfs

[Vivek Goyal]

On Wed, Feb 10, 2021 at 06:50:57PM -0500, Paul Moore wrote:
> On Tue, Feb 9, 2021 at 3:02 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > Now overlayfs allow unpriviliged mounts. That is root inside a non-init
> > user namespace can mount overlayfs. This was added in 5.10 kernel.

Actually this is being added in 5.11 kernel (and not 5.10 kernel).

Paul, can you please fix this while committing. If you want me to
report, let me know.

> >
> > Giuseppe tried to mount overlayfs with option "context" and it failed
> > with error -EACCESS.
> >
> > $ su test
> > $ unshare -rm
> > $ mkdir -p lower upper work merged
> > $ mount -t overlay -o lowerdir=lower,workdir=work,upperdir=upper,userxattr,context='system_u:object_r:container_file_t:s0' none merged
> >
> > This fails with -EACCESS. It works if option "-o context" is not specified.
> >
> > Little debugging showed that selinux_set_mnt_opts() returns -EACCESS.
> >
> > So this patch adds "overlay" to the list, where it is fine to specific
> > context from non init_user_ns.
> >
> > Reported-by: Giuseppe Scrivano <gscrivan@redhat.com>
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > ---
> >  security/selinux/hooks.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> This seems reasonable, but since we are at -rc7 this week it will need
> to wait until after the upcoming merge window.  It's too late in the
> cycle for new features.

I am fine with this going in 5.12 kernel. Thanks Paul.

Vivek

Re: [PATCH] selinux: Allow context mounts for unpriviliged overlayfs

[Paul Moore]

On Thu, Feb 11, 2021 at 9:01 AM Vivek Goyal <vgoyal@redhat.com> wrote:
> On Wed, Feb 10, 2021 at 06:50:57PM -0500, Paul Moore wrote:
> > On Tue, Feb 9, 2021 at 3:02 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > >
> > > Now overlayfs allow unpriviliged mounts. That is root inside a non-init
> > > user namespace can mount overlayfs. This was added in 5.10 kernel.
>
> Actually this is being added in 5.11 kernel (and not 5.10 kernel).
>
> Paul, can you please fix this while committing. If you want me to
> report, let me know.

Good to know, thanks for the clarification.  As far as updating the
commit description, while I generally prefer the patch author to make
changes (my personal opinion is that maintainers should have as light
a touch as possible outside the mechanical work of merging), this is
pretty minor and I can fix that up if you want.  Regardless, we've
likely got ~2.5 weeks before it really matters anyway :)

-- 
paul moore
www.paul-moore.com

Re: [PATCH] selinux: Allow context mounts for unpriviliged overlayfs

[Vivek Goyal]

On Thu, Feb 11, 2021 at 11:32:41AM -0500, Paul Moore wrote:
> On Thu, Feb 11, 2021 at 9:01 AM Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Wed, Feb 10, 2021 at 06:50:57PM -0500, Paul Moore wrote:
> > > On Tue, Feb 9, 2021 at 3:02 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > >
> > > > Now overlayfs allow unpriviliged mounts. That is root inside a non-init
> > > > user namespace can mount overlayfs. This was added in 5.10 kernel.
> >
> > Actually this is being added in 5.11 kernel (and not 5.10 kernel).
> >
> > Paul, can you please fix this while committing. If you want me to
> > report, let me know.
> 
> Good to know, thanks for the clarification.  As far as updating the
> commit description, while I generally prefer the patch author to make
> changes (my personal opinion is that maintainers should have as light
> a touch as possible outside the mechanical work of merging), this is
> pretty minor and I can fix that up if you want.  Regardless, we've
> likely got ~2.5 weeks before it really matters anyway :)

Ok, I will repost. Want to keep it as simple as possible for you. :-)

Vivek

> 
> -- 
> paul moore
> www.paul-moore.com
>