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

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

[Stephen Smalley]

Similar to what
https://lore.kernel.org/selinux/aqPUqU3eZeFrykj3@gremlin/ does for the
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.

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>
---
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);
 
 	return remap_pfn_range(vma, vma->vm_start,
 			       page_to_pfn(status),
-- 
2.55.0

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

[sashiko-bot]

> Similar to what
> https://lore.kernel.org/selinux/aqPUqU3eZeFrykj3@gremlin/ does for the
> 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.
> 
> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911145840.19039-2-stephen.smalley.work@gmail.com?part=1

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

[Lorenzo Stoakes (ARM)]

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

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

[Jan Kara]

On Fri 11-09-26 10:58:41, Stephen Smalley wrote:
> Similar to what
> https://lore.kernel.org/selinux/aqPUqU3eZeFrykj3@gremlin/ does for the
> 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.
> 
> 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>

Hum, so rather than having special checks in ->open, why don't you set
S_IMMUTABLE bit for the inodes which you really don't want anybody to
modify? That will block both writeable opens as well as truncate...

								Honza

> ---
> 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);
>  
>  	return remap_pfn_range(vma, vma->vm_start,
>  			       page_to_pfn(status),
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Stephen Smalley]

On Fri, Sep 11, 2026 at 11:23 AM Jan Kara <jack@suse.cz> wrote:
>
> On Fri 11-09-26 10:58:41, Stephen Smalley wrote:
> > Similar to what
> > https://lore.kernel.org/selinux/aqPUqU3eZeFrykj3@gremlin/ does for the
> > 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.
> >
> > 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>
>
> Hum, so rather than having special checks in ->open, why don't you set
> S_IMMUTABLE bit for the inodes which you really don't want anybody to
> modify? That will block both writeable opens as well as truncate...

Great idea. I will do that in v3, drop the .open checking, but still
remove the .mmap checking that will then be obsoleted, unless someone
objects?

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

[Jan Kara]

On Fri 11-09-26 12:14:55, Stephen Smalley wrote:
> On Fri, Sep 11, 2026 at 11:23 AM Jan Kara <jack@suse.cz> wrote:
> >
> > On Fri 11-09-26 10:58:41, Stephen Smalley wrote:
> > > Similar to what
> > > https://lore.kernel.org/selinux/aqPUqU3eZeFrykj3@gremlin/ does for the
> > > 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.
> > >
> > > 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>
> >
> > Hum, so rather than having special checks in ->open, why don't you set
> > S_IMMUTABLE bit for the inodes which you really don't want anybody to
> > modify? That will block both writeable opens as well as truncate...
> 
> Great idea. I will do that in v3, drop the .open checking, but still
> remove the .mmap checking that will then be obsoleted, unless someone
> objects?

That sounds good to me.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR