* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-11 16:37 [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks Stephen Smalley
@ 2026-09-11 16:57 ` sashiko-bot
2026-09-14 7:56 ` Jan Kara
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-11 16:57 UTC (permalink / raw)
Cc: selinux
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-11 16:37 [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks Stephen Smalley
2026-09-11 16:57 ` sashiko-bot
@ 2026-09-14 7:56 ` Jan Kara
2026-09-14 9:18 ` Lorenzo Stoakes (ARM)
2026-09-14 21:17 ` Paul Moore
3 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2026-09-14 7:56 UTC (permalink / raw)
To: Stephen Smalley
Cc: selinux, paul, omosnacek, ljs, jannh, jack, cgzones, brauner
On Fri 11-09-26 12:37:35, Stephen Smalley wrote:
> 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>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> 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
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-11 16:37 [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks Stephen Smalley
2026-09-11 16:57 ` sashiko-bot
2026-09-14 7:56 ` Jan Kara
@ 2026-09-14 9:18 ` Lorenzo Stoakes (ARM)
2026-09-14 12:39 ` Stephen Smalley
2026-09-14 21:17 ` Paul Moore
3 siblings, 1 reply; 11+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-14 9:18 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux, paul, omosnacek, jannh, jack, cgzones, brauner
On Fri, Sep 11, 2026 at 12:37:35PM -0400, Stephen Smalley wrote:
> 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
Micro nit: clear -> clears.
> 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>
With nits addressed, LGTM so:
Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> 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);
> + }
> +}
Hmm, maybe worth returning an error here on lookup failure? I think probably it
can't happen in reality right now but that'd make it consistent with the other
functions here.
> +
> 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
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-14 9:18 ` Lorenzo Stoakes (ARM)
@ 2026-09-14 12:39 ` Stephen Smalley
2026-09-14 12:45 ` Lorenzo Stoakes (ARM)
0 siblings, 1 reply; 11+ messages in thread
From: Stephen Smalley @ 2026-09-14 12:39 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: selinux, paul, omosnacek, jannh, jack, cgzones, brauner
On Mon, Sep 14, 2026 at 5:18 AM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
>
> On Fri, Sep 11, 2026 at 12:37:35PM -0400, Stephen Smalley wrote:
> > 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
>
> Micro nit: clear -> clears.
Thanks, will fix if Paul requests a re-spin.
>
> > 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>
>
> With nits addressed, LGTM so:
>
> Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>
> > ---
> > 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);
> > + }
> > +}
>
> Hmm, maybe worth returning an error here on lookup failure? I think probably it
> can't happen in reality right now but that'd make it consistent with the other
> functions here.
I don't think so - 1. It can't fail, and 2. if by some happenstance it
did fail, we would
NOT want to stop populating selinuxfs and return the error to the
caller because that
would turn something that is non-fatal into a fatal policy load/reload
error and halt
the system if this is the initial policy load and SELinux is
configured to be enforcing.
At most maybe a pr_err() or similar here, defer to Paul on whether that warrants
a re-spin. Thanks.
>
> > +
> > 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
> >
>
> --
> Cheers, Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-14 12:39 ` Stephen Smalley
@ 2026-09-14 12:45 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 11+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-14 12:45 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux, paul, omosnacek, jannh, jack, cgzones, brauner
On Mon, Sep 14, 2026 at 08:39:53AM -0400, Stephen Smalley wrote:
> On Mon, Sep 14, 2026 at 5:18 AM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
> >
> > On Fri, Sep 11, 2026 at 12:37:35PM -0400, Stephen Smalley wrote:
> > > 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
> >
> > Micro nit: clear -> clears.
>
> Thanks, will fix if Paul requests a re-spin.
>
> >
> > > 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>
> >
> > With nits addressed, LGTM so:
You can keep my tag without these 2 nits addressed actually, they are so minor
as to not matter really!
> >
> > Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> >
> > > ---
> > > 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);
> > > + }
> > > +}
> >
> > Hmm, maybe worth returning an error here on lookup failure? I think probably it
> > can't happen in reality right now but that'd make it consistent with the other
> > functions here.
>
> I don't think so - 1. It can't fail, and 2. if by some happenstance it
> did fail, we would
> NOT want to stop populating selinuxfs and return the error to the
> caller because that
> would turn something that is non-fatal into a fatal policy load/reload
> error and halt
> the system if this is the initial policy load and SELinux is
> configured to be enforcing.
> At most maybe a pr_err() or similar here, defer to Paul on whether that warrants
> a re-spin. Thanks.
Yup not a big deal!
As above, you can keep the tag regardless of these nits being addressed :)
>
> >
> > > +
> > > 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
> > >
> >
> > --
> > Cheers, Lorenzo
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-11 16:37 [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks Stephen Smalley
` (2 preceding siblings ...)
2026-09-14 9:18 ` Lorenzo Stoakes (ARM)
@ 2026-09-14 21:17 ` Paul Moore
2026-09-15 9:15 ` Jan Kara
2026-09-15 12:41 ` Stephen Smalley
3 siblings, 2 replies; 11+ messages in thread
From: Paul Moore @ 2026-09-14 21:17 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux, omosnacek, ljs, jannh, jack, cgzones, brauner
On Fri, Sep 11, 2026 at 12:38 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> 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(-)
I'm mildly concerned about marking these files as immutable when
neither of them is actually immutable. Yes, userspace shouldn't be
able to write to either, but the contents of these files *do* change.
While it doesn't look S_IMMUTABLE should currently pose any real
problems, I do worry about the future and an innocent looking VFS
change causing problems with one of these files because it was assumed
the contents of a S_IMMUTABLE file wouldn't change.
Unless someone can promise me that would never happen, I think I would
prefer the !FMODE_WRITE approach taken in v2.
--
paul-moore.com
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-14 21:17 ` Paul Moore
@ 2026-09-15 9:15 ` Jan Kara
2026-09-15 13:01 ` Christian Brauner
2026-09-15 21:28 ` Paul Moore
2026-09-15 12:41 ` Stephen Smalley
1 sibling, 2 replies; 11+ messages in thread
From: Jan Kara @ 2026-09-15 9:15 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, selinux, omosnacek, ljs, jannh, jack, cgzones,
brauner
On Mon 14-09-26 17:17:39, Paul Moore wrote:
> On Fri, Sep 11, 2026 at 12:38 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > 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(-)
>
> I'm mildly concerned about marking these files as immutable when
> neither of them is actually immutable. Yes, userspace shouldn't be
> able to write to either, but the contents of these files *do* change.
> While it doesn't look S_IMMUTABLE should currently pose any real
> problems, I do worry about the future and an innocent looking VFS
> change causing problems with one of these files because it was assumed
> the contents of a S_IMMUTABLE file wouldn't change.
S_IMMUTABLE bit is about permission checks. I have hard time imagining
some VFS code would do some assumptions regarding data based on that. But
maybe my imagination is just lacking :).
> Unless someone can promise me that would never happen, I think I would
> prefer the !FMODE_WRITE approach taken in v2.
If you feel better that way, then sure, go for it. It works as well but
you have to separately handle truncate(2). Overall I wouldn't consider this
less fragile than using S_IMMUTABLE but it's reasonably robust as well.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-15 9:15 ` Jan Kara
@ 2026-09-15 13:01 ` Christian Brauner
2026-09-15 21:28 ` Paul Moore
1 sibling, 0 replies; 11+ messages in thread
From: Christian Brauner @ 2026-09-15 13:01 UTC (permalink / raw)
To: Jan Kara
Cc: Paul Moore, Stephen Smalley, selinux, omosnacek, ljs, jannh,
cgzones
On Tue, Sep 15, 2026 at 11:15:43AM +0200, Jan Kara wrote:
> On Mon 14-09-26 17:17:39, Paul Moore wrote:
> > On Fri, Sep 11, 2026 at 12:38 PM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > >
> > > 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(-)
> >
> > I'm mildly concerned about marking these files as immutable when
> > neither of them is actually immutable. Yes, userspace shouldn't be
> > able to write to either, but the contents of these files *do* change.
> > While it doesn't look S_IMMUTABLE should currently pose any real
> > problems, I do worry about the future and an innocent looking VFS
> > change causing problems with one of these files because it was assumed
> > the contents of a S_IMMUTABLE file wouldn't change.
>
> S_IMMUTABLE bit is about permission checks. I have hard time imagining
> some VFS code would do some assumptions regarding data based on that. But
> maybe my imagination is just lacking :).
Said code would be rather buggy...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-15 9:15 ` Jan Kara
2026-09-15 13:01 ` Christian Brauner
@ 2026-09-15 21:28 ` Paul Moore
1 sibling, 0 replies; 11+ messages in thread
From: Paul Moore @ 2026-09-15 21:28 UTC (permalink / raw)
To: Jan Kara
Cc: Stephen Smalley, selinux, omosnacek, ljs, jannh, cgzones, brauner
On Tue, Sep 15, 2026 at 5:15 AM Jan Kara <jack@suse.cz> wrote:
> On Mon 14-09-26 17:17:39, Paul Moore wrote:
> > On Fri, Sep 11, 2026 at 12:38 PM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > >
> > > 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(-)
> >
> > I'm mildly concerned about marking these files as immutable when
> > neither of them is actually immutable. Yes, userspace shouldn't be
> > able to write to either, but the contents of these files *do* change.
> > While it doesn't look S_IMMUTABLE should currently pose any real
> > problems, I do worry about the future and an innocent looking VFS
> > change causing problems with one of these files because it was assumed
> > the contents of a S_IMMUTABLE file wouldn't change.
>
> S_IMMUTABLE bit is about permission checks. I have hard time imagining
> some VFS code would do some assumptions regarding data based on that. But
> maybe my imagination is just lacking :).
>
> > Unless someone can promise me that would never happen, I think I would
> > prefer the !FMODE_WRITE approach taken in v2.
>
> If you feel better that way, then sure, go for it. It works as well but
> you have to separately handle truncate(2). Overall I wouldn't consider this
> less fragile than using S_IMMUTABLE but it's reasonably robust as well.
Okay, since my concerns are only a gut feeling and the S_IMMUTABLE
patch does take care of a few issues at once, I'm going to go ahead
and merge this into selinux/dev; I just hope I don't end up regretting
this in a few years ;)
Thanks everyone!
--
paul-moore.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] selinux: mark status and policy inodes as immutable, drop status mmap write checks
2026-09-14 21:17 ` Paul Moore
2026-09-15 9:15 ` Jan Kara
@ 2026-09-15 12:41 ` Stephen Smalley
1 sibling, 0 replies; 11+ messages in thread
From: Stephen Smalley @ 2026-09-15 12:41 UTC (permalink / raw)
To: Paul Moore; +Cc: selinux, omosnacek, ljs, jannh, jack, cgzones, brauner
On Mon, Sep 14, 2026 at 5:17 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Fri, Sep 11, 2026 at 12:38 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > 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(-)
>
> I'm mildly concerned about marking these files as immutable when
> neither of them is actually immutable. Yes, userspace shouldn't be
> able to write to either, but the contents of these files *do* change.
> While it doesn't look S_IMMUTABLE should currently pose any real
> problems, I do worry about the future and an innocent looking VFS
> change causing problems with one of these files because it was assumed
> the contents of a S_IMMUTABLE file wouldn't change.
>
> Unless someone can promise me that would never happen, I think I would
> prefer the !FMODE_WRITE approach taken in v2.
In that case, we would still need to separately address truncate(2) in
some manner, which requires something like cgzones' earlier patch. Up
to you but the immutable flag seems cleaner to me and I don't think
the concern above is likely to be a real issue.
^ permalink raw reply [flat|nested] 11+ messages in thread