Re: [PATCH 31/39] convert selinuxfs
From: Paul Moore <paul@paul-moore.com>
Date: 2025-09-21 20:44:40
Also in:
linux-efi, linux-fsdevel, linux-mm, linux-usb, ocfs2-devel
On Sat, Sep 20, 2025 at 3:48 AM Al Viro [off-list ref] 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. Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- security/selinux/selinuxfs.c | 52 +++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 22 deletions(-)
...
quoted hunk ↗ jump to hunk
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 9aa1d03ab612..dc1bb49664f2 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c@@ -1966,10 +1973,11 @@ static struct dentry *sel_make_swapover_dir(struct super_block *sb, /* directory inodes start off with i_nlink == 2 (for "." entry) */ inc_nlink(inode); inode_lock(sb->s_root->d_inode); - d_add(dentry, inode); + d_make_persistent(dentry, inode); inc_nlink(sb->s_root->d_inode); inode_unlock(sb->s_root->d_inode); - return dentry; + dput(dentry); + return dentry; // borrowed }
Prefer C style comments on their own line: dput(dentry); /* borrowed dentry */ return dentry;
quoted hunk ↗ jump to hunk
@@ -2079,15 +2088,14 @@ 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, + dentry = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME, &fsi->last_ino);
I'd probably keep fsi->policycap_dir in this patch simply to limit the scope of this patch to just the DCACHE_PERSISTENT related changes, but I'm not going to make a big fuss about that.
- if (IS_ERR(fsi->policycap_dir)) {
- ret = PTR_ERR(fsi->policycap_dir);
- fsi->policycap_dir = NULL;
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
goto err;
}
- ret = sel_make_policycap(fsi);
+ ret = sel_make_policycap(fsi, dentry);
if (ret) {
pr_err("SELinux: failed to load policy capabilities\n");
goto err;-- paul-moore.com