[PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks

[PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks

[Stephen Smalley]

The selinuxfs "status" and "policy" files are read-only interfaces
that are also mmap'd by userspace. They are created 0444 by
simple_fill_super() but a CAP_DAC_OVERRIDE caller can still open them
O_WRONLY/O_RDWR, open(O_RDONLY|O_TRUNC) them, or truncate(2) them.

Mark both inodes S_IMMUTABLE at fill_super time. inode_permission()
tests IS_IMMUTABLE before the DAC / capability checks, so all of the
above are rejected at the VFS layer without ever reaching the file
operations. Since a writable file can no longer exist, do_mmap() clear
VM_MAYWRITE for MAP_SHARED mappings on its own, and the
sel_mmap_handle_status() write/mprotect guards are dead; drop them.
MAP_PRIVATE writable mappings become permitted (they were previously
-EPERM) and CoW harmlessly to a private page, matching how
sel_mmap_policy() has always treated the private case.

The sel_mmap_policy() VM_SHARED guard becomes redundant for the same
reason; leave dropping it to the pending "selinux: reject writable
opens of policy file, drop mmap shared/write check" patch so that the
patches do not conflict.

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
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
v3 implements Jan's suggestion to mark the inodes immutable rather than
implementing open-time checks.

 security/selinux/selinuxfs.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 292302eb60f3..0941ce79ea0b 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -247,11 +247,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),
@@ -1818,6 +1813,17 @@ static struct dentry *sel_make_swapover_dir(struct super_block *sb, u64 *ino)
 
 #define NULL_FILE_NAME "null"
 
+static void sel_mark_immutable(struct dentry *root, const char *name)
+{
+	struct qstr q = QSTR(name);
+	struct dentry *dentry = try_lookup_noperm(&q, root);
+
+	if (!IS_ERR_OR_NULL(dentry)) {
+		d_inode(dentry)->i_flags |= S_IMMUTABLE;
+		dput(dentry);
+	}
+}
+
 static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct selinux_fs_info *fsi;
@@ -1857,6 +1863,9 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (ret)
 		goto err;
 
+	sel_mark_immutable(sb->s_root, "status");
+	sel_mark_immutable(sb->s_root, "policy");
+
 	fsi = sb->s_fs_info;
 	fsi->bool_dir = sel_make_dir(sb->s_root, BOOL_DIR_NAME, &fsi->last_ino);
 	if (IS_ERR(fsi->bool_dir)) {
-- 
2.55.0

Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks

[sashiko-bot]

> The selinuxfs "status" and "policy" files are read-only interfaces
> that are also mmap'd by userspace. They are created 0444 by
> simple_fill_super() but a CAP_DAC_OVERRIDE caller can still open them
> O_WRONLY/O_RDWR, open(O_RDONLY|O_TRUNC) them, or truncate(2) them.
> 
> Mark both inodes S_IMMUTABLE at fill_super time. inode_permission()
> tests IS_IMMUTABLE before the DAC / capability checks, so all of the
> above are rejected at the VFS layer without ever reaching the file
> operations. Since a writable file can no longer exist, do_mmap() clear
> VM_MAYWRITE for MAP_SHARED mappings on its own, and the
> sel_mmap_handle_status() write/mprotect guards are dead; drop them.
> MAP_PRIVATE writable mappings become permitted (they were previously
> -EPERM) and CoW harmlessly to a private page, matching how
> sel_mmap_policy() has always treated the private case.
> 
> The sel_mmap_policy() VM_SHARED guard becomes redundant for the same
> reason; leave dropping it to the pending "selinux: reject writable
> opens of policy file, drop mmap shared/write check" patch so that the
> patches do not conflict.
> 
> 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
> Suggested-by: Jan Kara <jack@suse.cz>
> 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/20260911163734.22981-2-stephen.smalley.work@gmail.com?part=1