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) <[email protected]> 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) <[email protected]> > > 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 <[email protected]> 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
