[PATCH 31/39] convert selinuxfs

[Stephen Smalley]

On Sun, Sep 21, 2025 at 10:45 PM Paul Moore <paul at paul-moore.com> wrote:
>
> On Sun, Sep 21, 2025 at 5:41 PM Al Viro <viro at zeniv.linux.org.uk> wrote:
> > On Sun, Sep 21, 2025 at 04:44:28PM -0400, Paul Moore wrote:
> > > > +       dput(dentry);
> > > > +       return dentry;  // borrowed
> > > >  }
> > >
> > > Prefer C style comments on their own line:
> > >
> > >   dput(dentry);
> > >   /* borrowed dentry */
> > >   return dentry;
> >
> > Umm...  IMO that's more of an annotation along the lines of "fallthru"...
>
> Maybe, I still prefer the example provided above.  The heart wants
> what the heart wants I guess.
>
> > > > @@ -2079,15 +2088,14 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
> > > >                 goto err;
> > > >         }
> > > >
> > > > -       fsi->policycap_dir = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
> > > > +       dentry = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
> > > >                                           &fsi->last_ino);
> > >
> > > I'd probably keep fsi->policycap_dir in this patch simply to limit the
> > > scope of this patch to just the DCACHE_PERSISTENT related changes, but
> > > I'm not going to make a big fuss about that.
> >
> > Not hard to split that way.  Will do...
>
> Thanks.
>
> > BTW, an unrelated question: does userland care about selinuxfs /null being
> > called that (and being on selinuxfs, for that matter)?  Same for the
> > apparmor's securityfs /apparmor/.null...
>
> That's an interesting question.  The kernel really only references it
> in one place after creation, and as you've already seen, that's easily
> changed.  It's more important that it can be uniquely labeled such
> that most any process can open it, otherwise we run into problems when
> trying to replace fds when another file that the process can't open.
>
> I'm adding the SELinux list to tap into the folks that play with
> userland more than I do, but off the top of my head I can't think of
> why userspace would need to do anything directly with
> /sys/fs/selinux/null.  There are some comments in the userland code
> about not being able to mount selinuxfs with MS_NODEV due to the null
> device, but that's the only obvious thing I see while quickly
> searching through the code tonight.

Is there a reason why these patches weren't sent to selinux list in
the first place?
In any event, yes, Android userspace (in particular the Android init
program) relies on /sys/fs/selinux/null at a point where /dev/null
does not yet exist [1]. Hence, I don't believe we can drop it since it
would break userspace.

[1] https://cs.android.com/search?q=%2Fsys%2Ffs%2Fselinux%2Fnull&sq=&ss=android%2Fplatform%2Fsuperproject%2Fmain


>
> > If nobody cares, I would rather add an internal-only filesystem with
> > root being a character device (1,3) and whatever markings selinux et.al.
> > need for it.  With open_devnull(creds) provided for selinux,
> > apparmor and whoever else wants to play with neutering descriptors
> > on exec...
>
> With the ongoing efforts to push towards better support for multiple
> simultaneous LSMs, we would likely need to make sure there each LSM
> that currently has their own null device would continue to have their
> own, otherwise we potentially run into permission conflicts between
> LSMs where one could end up blocking another and then we're back to
> not having a file to use as a replacement.  Not sure if that is what
> you had in mind with your proposal, but just wanted to make sure that
> was factored into the idea.
>
> --
> paul-moore.com
>