Re: [PATCH v2] selinux: preserve user SID across nested backing files

[Amir Goldstein]

On Thu, Aug 27, 2026 at 9:39 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Aug 20, 2026 Karl Mehltretter <kmehltretter@xxxxxxxxx> wrote:
> >
> > SELinux saves the user file SID in a backing-file security blob so it
> > remains available after mmap() replaces vma->vm_file with a backing file.
> >
> > For nested backing files (overlayfs over overlayfs, or FUSE passthrough
> > backed by overlayfs), user_file may itself be a backing file.  Its
> > fsec->sid is the SID of the mounter that opened it, rather than the user
> > that opened the top-level file.  mprotect() then checks fd { use } against
> > the mounter SID.  This can incorrectly deny access without a domain
> > transition, or check the wrong target SID after one.
> >
> > Copy the saved user SID when user_file is a backing file.  Keep using the
> > regular file SID for the first backing layer.
> >
> > With two nested overlayfs mounts and SELinux enforcing,
> > mprotect(PROT_READ) returns EACCES with an fd { use } denial against the
> > mounter SID.  With this change, mprotect() succeeds.
> >
> > Fixes: 82544d36b172 ("selinux: fix overlayfs mmap() and mprotect() access checks")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Assisted-by: Codex:gpt-5.6-sol
> > Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>
> > Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> > Changes in v2:
> > - Add selinux_file_user_sid() helper instead of open-coding the lookup
> >   (Amir).
> >
> > Tested on arm64 QEMU at fd6e2388a3ea with SELinux enforcing and two
> > nested overlayfs mounts. The policy omitted only
> > base_t -> mounter_t:fd { use } among the relevant cross-domain allows:
> >
> >   baseline: mprotect(PROT_READ) returned EACCES with that denial
> >   patched:  mprotect(PROT_READ) succeeded; test exited 0
>
> If you could share the distro and some more details on the testing
> methodology that would be helpful.
>
> > This patch fixes SID propagation only. backing_file_user_path() still
> > resolves to the middle layer for a nested mapping, so the audit path and
> > inode do not correspond to uf_sid, and that layer's mounter is not
> > re-checked. Preserving the full user path likely needs a VFS-side change,
> > such as having backing_file_open() store file_user_path(user_file).
>
> Additional comments below, and while this patch is useful in fixing some
> of the problems (thank you!), we obviously need to fix the others as
> well.  Is this something you think you will be able to work on?
>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 1ead2eee1944..171b90412ff1 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -3843,13 +3843,20 @@ static int selinux_file_alloc_security(struct file *file)
> >       return 0;
> >  }
> >
> > +static inline u32 selinux_file_user_sid(const struct file *file)
> > +{
> > +     if (unlikely(file->f_mode & FMODE_BACKING))
> > +             return selinux_backing_file(file)->uf_sid;
> > +     return selinux_file(file)->sid;
> > +}
>
> I'm a little concerned that this only works for one additional level of
> filesystem stacking.  Yes, I know that OVL_MAX_NESTING and
> FILESYSTEM_MAX_STACK_DEPTH are currently set at "2", but it's not an
> unreasonable concern that at some point in the future that number will
> increase without proper notification or testing and we will once again
> have a problem.
>
> At the absolute minimum we should have a BUILD_BUG_ON() for the stacking
> depth.  It would be good if we could watch both the overlayfs and vfs
> constants, but the overlayfs constant isn't available outside
> fs/overlayfs/inode.c (thankfully it is currently set to the vfs limit).
>
>   BUIILD_BUG_ON(FILESYSTEM_MAX_STACK_DEPTH > 2);
>
> Ideally, the code would be written to keep diving down the stack until
> it hit the true backing file.  No one likes to see recursion, but at the
> point where this function is called there should already be a reasonable
> bound on the stacking depth and the work involved.

Unless I am missing something, stack depth should not matter.
file_user_path() should hold the user visible path, for all the backing files
in all the depths of the backing files stack.

This is the issue that was fixed with commit
f2381b546e7e6 fs: fix user path of nested backing files

This patch makes uf_sid behave the same way and also documents this:

struct backing_file_security_struct {
-       u32 uf_sid; /* associated user file fsec->sid */
+       u32 uf_sid; /* top-level user file fsec->sid */
 };

[*] For me the "user" visible file is always the "top-level" but if
this comnment
    change helps others understand better it's fine by me.

Thanks,
Amir.