Re: [PATCH v2] selinux: reject writable opens of status file, drop mmap write checks

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Fri, Sep 11, 2026 at 10:58:41AM -0400, Stephen Smalley wrote:
> Similar to what
> https://lore.kernel.org/selinux/aqPUqU3eZeFrykj3@gremlin/ does for the

NIT: Probably better to have that as a Link: tag ?

> policy file, update the .open handler for the status file to reject
> writable opens, and update the .mmap handler to stop clearing
> VM_MAYWRITE and checking for VM_WRITE. This does NOT prevent
> truncation via open(O_RDONLY|O_TRUNC) or truncate(), which is
> left to another patch.

Makes sense!

>
> Link: https://lore.kernel.org/selinux/aqPUqU3eZeFrykj3@gremlin/
> cc: ljs@kernel.org
> cc: jannh@google.com
> cc: jack@suse.cz
> cc: cgzones@googlemail.com
> cc: brauner@kernel.org
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Seems reasonable to me so:

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
> v2 corrects my previous incorrect claim re preventing truncation,
> deferring that to a separate patch like the still-pending one
> from cgzones, and also moves the FMODE_WRITE check before the
> status page allocation.
>
>  security/selinux/selinuxfs.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 292302eb60f3..835594d221fc 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -214,8 +214,12 @@ static const struct file_operations sel_handle_unknown_ops = {
>
>  static int sel_open_handle_status(struct inode *inode, struct file *filp)
>  {
> -	struct page    *status = selinux_kernel_status_page();
> +	struct page    *status;
>
> +	if (filp->f_mode & FMODE_WRITE)
> +		return -EACCES;
> +
> +	status = selinux_kernel_status_page();
>  	if (!status)
>  		return -ENOMEM;
>
> @@ -247,11 +251,6 @@ static int sel_mmap_handle_status(struct file *filp,
>  	/* only allows one page from the head */
>  	if (vma->vm_pgoff > 0 || size != PAGE_SIZE)
>  		return -EIO;
> -	/* disallow writable mapping */
> -	if (vma->vm_flags & VM_WRITE)
> -		return -EPERM;
> -	/* disallow mprotect() turns it into writable */
> -	vm_flags_clear(vma, VM_MAYWRITE);

A note for any curious as to why I didn't touch this in my patch - it's because
it's a PFN remap.

This means it's a kernel-owned mapping, where dropping VMA_MAYWRITE_BIT is
permitted, so it wouldn't have fallen foul of my series' checks which disallow
this for non-kernel-owned mappings.

>
>  	return remap_pfn_range(vma, vma->vm_start,
>  			       page_to_pfn(status),
> --
> 2.55.0
>

--
Cheers, Lorenzo