Re: [PATCH 11/39] selinux: reject writable opens of policy file, drop mmap shared/write check

[Lorenzo Stoakes (ARM)]

On Thu, Sep 10, 2026 at 02:11:18PM -0400, Stephen Smalley wrote:
> On Tue, Sep 8, 2026 at 4:07 PM Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx> wrote:
> >
> > The policy file has no write method and is exposed read-only (S_IRUGO in
> > selinux_files[]), yet sel_open_policy() performs no open mode check, so a
> > CAP_DAC_OVERRIDE caller can open it O_RDWR. Reject FMODE_WRITE at open, as
> > kernfs does.
> >
> > The file can then never be mapped with FMODE_WRITE, so do_mmap() always
> > clears VM_MAYWRITE and VM_SHARED for MAP_SHARED mappings and the VM_SHARED
> > check in sel_mmap_policy() cannot be reached. Remove it.
> >
> > This also stops sel_mmap_policy() clearing VM_MAYWRITE on a mapping that is
> > neither a PFN map nor a mixed map, ahead of the core enforcing that only
> > such mappings may do so.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
>
> We should likely do something similar for the other sel_open_*() and
> sel_mmap_*() functions too.
> In particular, truncation of the status file can bring down systemd
> since it is mapped by libselinux.

Yeah I think there's definitely more that can be done there.

Though I think obviously that kind of stuff is better as a follow up as this
patch is focused on eliminating a case where VMA_MAYWRITE_BIT is cleared
incorrectly.

>
> Acked-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>

Thanks!
>
> > ---
> >  security/selinux/selinuxfs.c | 11 +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> > index c7d91476971c..545a6f89f9e7 100644
> > --- a/security/selinux/selinuxfs.c
> > +++ b/security/selinux/selinuxfs.c
> > @@ -340,6 +340,9 @@ static int sel_open_policy(struct inode *inode, struct file *filp)
> >         struct policy_load_memory *plm = NULL;
> >         int rc;
> >
> > +       if (filp->f_mode & FMODE_WRITE)
> > +               return -EACCES;
> > +
> >         rc = avc_has_perm(current_sid(), SECINITSID_SECURITY,
> >                           SECCLASS_SECURITY, SECURITY__READ_POLICY, NULL);
> >         if (rc)
> > @@ -424,14 +427,6 @@ static const struct vm_operations_struct sel_mmap_policy_ops = {
> >
> >  static int sel_mmap_policy(struct file *filp, struct vm_area_struct *vma)
> >  {
> > -       if (vma->vm_flags & VM_SHARED) {
> > -               /* do not allow mprotect to make mapping writable */
> > -               vm_flags_clear(vma, VM_MAYWRITE);
> > -
> > -               if (vma->vm_flags & VM_WRITE)
> > -                       return -EACCES;
> > -       }
> > -
> >         vm_flags_set(vma, VM_DONTEXPAND | VM_DONTDUMP);
> >         vma->vm_ops = &sel_mmap_policy_ops;
> >
> >
> > --
> > 2.55.0
> >

--
Cheers, Lorenzo