On Sat, Sep 20, 2025 at 3:48 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> > Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx> > --- > security/selinux/selinuxfs.c | 52 +++++++++++++++++++++--------------- > 1 file changed, 30 insertions(+), 22 deletions(-) ... > 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; > @@ -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