Re: [PATCH 31/39] convert selinuxfs

Al Viro
2025-09-21T22:26:25Z

On Sun, Sep 21, 2025 at 04:44:28PM -0400, Paul Moore wrote:
> On Sat, Sep 20, 2025 at 3:48 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > Tree has invariant part + two subtrees that get replaced upon each
> > policy load. Invariant parts stay for the lifetime of filesystem,
> > these two subdirs - from policy load to policy load (serialized
> > on lock_rename(root, ...)).
> >
> > All object creations are via d_alloc_name()+d_add() inside selinuxfs,
> > all removals are via simple_recursive_removal().
> >
> > Turn those d_add() into d_make_persistent()+dput() and that's mostly it.
> > Don't bother to store the dentry of /policy_capabilities - it belongs
> > to invariant part of tree and we only use it to populate that directory,
> > so there's no reason to keep it around afterwards.
>
> Minor comment on that below, as well as a comment style nitpick, but
> overall no major concerns from me.

FWIW, how's this for the preparatory part?

commit 17f3b70a28233078dd3dae3cf773b68fcd899950
Author: Al Viro <viro@zeniv.linux.org.uk>
Date: Sun Sep 21 18:09:48 2025 -0400

selinuxfs: don't stash the dentry of /policy_capabilities

Don't bother to store the dentry of /policy_capabilities - it belongs
to invariant part of tree and we only use it to populate that directory,
so there's no reason to keep it around afterwards.

Same situation as with /avc, /ss, etc. There are two directories that
get replaced on policy load - /class and /booleans. These we need to
stash (and update the pointers on policy reload); /policy_capabilities
is not in the same boat.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 9aa1d03ab612..482a2cac9640 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -75,7 +75,6 @@ struct selinux_fs_info {
struct dentry *class_dir;
unsigned long last_class_ino;
bool policy_opened;
- struct dentry *policycap_dir;
unsigned long last_ino;
struct super_block *sb;
};
@@ -117,7 +116,6 @@ static void selinux_fs_info_free(struct super_block *sb)

#define BOOL_DIR_NAME "booleans"
#define CLASS_DIR_NAME "class"
-#define POLICYCAP_DIR_NAME "policy_capabilities"

#define TMPBUFLEN 12
static ssize_t sel_read_enforce(struct file *filp, char __user *buf,
@@ -1879,23 +1877,24 @@ static int sel_make_classes(struct selinux_policy *newpolicy,
return rc;
}

-static int sel_make_policycap(struct selinux_fs_info *fsi)
+static int sel_make_policycap(struct dentry *dir)
{
+ struct super_block *sb = dir->d_sb;
unsigned int iter;
struct dentry *dentry = NULL;
struct inode *inode = NULL;

for (iter = 0; iter <= POLICYDB_CAP_MAX; iter++) {
if (iter < ARRAY_SIZE(selinux_policycap_names))
- dentry = d_alloc_name(fsi->policycap_dir,
+ dentry = d_alloc_name(dir,
selinux_policycap_names[iter]);
else
- dentry = d_alloc_name(fsi->policycap_dir, "unknown");
+ dentry = d_alloc_name(dir, "unknown");

if (dentry == NULL)
return -ENOMEM;

- inode = sel_make_inode(fsi->sb, S_IFREG | 0444);
+ inode = sel_make_inode(sb, S_IFREG | 0444);
if (inode == NULL) {
dput(dentry);
return -ENOMEM;
@@ -2079,15 +2078,13 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
goto err;
}

- fsi->policycap_dir = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
- &fsi->last_ino);
- if (IS_ERR(fsi->policycap_dir)) {
- ret = PTR_ERR(fsi->policycap_dir);
- fsi->policycap_dir = NULL;
+ dentry = sel_make_dir(sb->s_root, "policy_capabilities", &fsi->last_ino);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
goto err;
}

- ret = sel_make_policycap(fsi);
+ ret = sel_make_policycap(dentry);
if (ret) {
pr_err("SELinux: failed to load policy capabilities\n");
goto err;