| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5893d15993a809a3b1b80deb0c27bf1e@paul-moore.com>
Date: Fri, 30 Jan 2026 15:46:23 -0500
From: Paul Moore <paul@...l-moore.com>
To: Christian Göttsche <cgoettsche@...tendoof.de>, selinux@...r.kernel.org
Cc: Christian Göttsche <cgzones@...glemail.com>, Stephen Smalley <stephen.smalley.work@...il.com>, Ondrej Mosnacek <omosnace@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] selinux: prevent truncation of status map
On Jan 30, 2026 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@...tendoof.de> wrote:
>
> Currently the SELinux status map can be truncated, given the necessary
> permissions, leading to foreign user space processes getting a bus error
> (SIGBUS) while concurrently making use of the status map.
> For example systemd can be killed that way, see [1].
>
> Override the setattr inode handler and check for O_TRUNC in the open
> handler to prevent truncations.
>
> Link [1]: https://github.com/systemd/systemd/issues/37349
>
> Closes: https://github.com/SELinuxProject/selinux/issues/475
> Signed-off-by: Christian Göttsche <cgzones@...glemail.com>
> ---
> security/selinux/selinuxfs.c | 43 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 896acad1f5f7..df079a35a02d 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -214,10 +214,30 @@ static const struct file_operations sel_handle_unknown_ops = {
> .llseek = generic_file_llseek,
> };
>
> +static int sel_setattr_handle_status(struct mnt_idmap *idmap,
> + struct dentry *dentry,
> + struct iattr *iattr)
> +{
> + /* Prevent truncation to avoid raising SIGBUS */
> + if (iattr->ia_valid & ATTR_SIZE)
> + return -EINVAL;
Do we want this as -EINVAL or -EPERM? However, see my comments below
about how to handle the ATTR_SIZE case.
> + return simple_setattr(idmap, dentry, iattr);
> +}
> +
> +static const struct inode_operations sel_handle_status_iops = {
> + .setattr = sel_setattr_handle_status,
> +};
> +
> static int sel_open_handle_status(struct inode *inode, struct file *filp)
> {
> - struct page *status = selinux_kernel_status_page();
> + struct page *status;
>
> + /* Prevent truncation to avoid raising SIGBUS */
> + if (filp->f_flags & O_TRUNC)
> + return -EINVAL;
Same as above, should this be -EPERM?
> + status = selinux_kernel_status_page();
> if (!status)
> return -ENOMEM;
>
> @@ -1980,7 +2000,6 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
> [SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
> [SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, S_IRUGO},
> [SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, S_IRUGO},
> - [SEL_STATUS] = {"status", &sel_handle_status_ops, S_IRUGO},
> [SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO},
> [SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops,
> S_IWUGO},
> @@ -1995,6 +2014,26 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
> if (ret)
> goto err;
>
> + /* Create "status" separately to assign a custom inode_operations */
> + {
> + ret = -ENOMEM;
> +
> + dentry = d_alloc_name(sb->s_root, "status");
> + if (!dentry)
> + goto err;
> + inode = new_inode(sb);
> + if (!inode) {
> + dput(dentry);
> + goto err;
> + }
> + inode->i_mode = S_IFREG | 0444;
> + simple_inode_init_ts(inode);
> + inode->i_fop = &sel_handle_status_ops;
> + inode->i_op = &sel_handle_status_iops;
> + inode->i_ino = SEL_STATUS;
> + d_add(dentry, inode);
> + }
I worry a little about duplicating and open coding the per-file loop from
simple_fill_super(), I can see things slowly getting out of sync and bad
things happening. Unfortunately, I don't see anything in libfs that would
allow us to supply our own inode_operations either.
What do you think about using selinux_inode_setattr() as shown below,
would this work?
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index feda34b18d83..8e4374f22a18 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -106,6 +106,7 @@
#include "netlabel.h"
#include "audit.h"
#include "avc_ss.h"
+#include "selinuxfs.h"
#define SELINUX_INODE_INIT_XATTRS 1
@@ -3291,6 +3292,12 @@ static int selinux_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
return 0;
}
+ if (inode->i_sb->s_magic == SELINUX_MAGIC) {
+ int rc = selinuxfs_inode_setattr(idmap, inode, iattr);
+ if (rc)
+ return rc;
+ }
+
if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID |
ATTR_ATIME_SET | ATTR_MTIME_SET | ATTR_TIMES_SET))
return dentry_has_perm(cred, dentry, FILE__SETATTR);
diff --git a/security/selinux/include/selinuxfs.h b/security/selinux/include/selinuxfs.h
new file mode 100644
index 000000000000..f5e6da02833b
--- /dev/null
+++ b/security/selinux/include/selinuxfs.h
@@ -0,0 +1,2 @@
+int selinuxfs_inode_setattr(struct mnt_idmap *idmap, struct inode *inode,
+ struct iattr *iattr);
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 896acad1f5f7..5af5e1dee743 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -43,6 +43,7 @@
#include "objsec.h"
#include "conditional.h"
#include "ima.h"
+#include "selinuxfs.h"
enum sel_inos {
SEL_ROOT_INO = 2,
@@ -1954,6 +1955,20 @@ static struct dentry *sel_make_swapover_dir(struct super_block *sb,
return dentry; // borrowed
}
+int selinuxfs_inode_setattr(struct mnt_idmap *idmap, struct inode *inode,
+ struct iattr *iattr)
+{
+ switch (inode->i_ino) {
+ case SEL_STATUS:
+ if (iattr->ia_valid & ATTR_SIZE)
+ return -EPERM;
+ default:
+ break;
+ }
+
+ return 0;
+}
--
paul-moore.com
Powered by blists - more mailing lists