From b1494e6bc47c18df9c759445fb03764a3bdb7ec3 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 9 Mar 2025 23:54:44 -0400 Subject: configfs, securityfs: kill_litter_super() not needed These are guaranteed to be empty by the time they are shut down; both are single-instance and there is an internal mount maintained for as long as there is any contents. Both have that internal mount pinned by every object in root. In other words, kill_litter_super() boils down to kill_anon_super() for those. Reviewed-by: Joel Becker Acked-by: Paul Moore (LSM) Acked-by: Andreas Hindborg (configfs) Signed-off-by: Al Viro --- security/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/inode.c b/security/inode.c index 43382ef8896e..bf7b5e2e6955 100644 --- a/security/inode.c +++ b/security/inode.c @@ -70,7 +70,7 @@ static struct file_system_type fs_type = { .owner = THIS_MODULE, .name = "securityfs", .init_fs_context = securityfs_init_fs_context, - .kill_sb = kill_litter_super, + .kill_sb = kill_anon_super, }; /** -- cgit v1.2.3 From bdd80b5c1b35eb727a5065369ae5f45791f2a9cd Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 26 Feb 2024 02:07:09 -0500 Subject: convert smackfs Entirely static tree populated by simple_fill_super(). Can use kill_anon_super() as-is. Acked-by: Casey Schaufler Signed-off-by: Al Viro --- security/smack/smackfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index b1e5e62f5cbd..e989ae3890c7 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -2960,7 +2960,7 @@ static int smk_init_fs_context(struct fs_context *fc) static struct file_system_type smk_fs_type = { .name = "smackfs", .init_fs_context = smk_init_fs_context, - .kill_sb = kill_litter_super, + .kill_sb = kill_anon_super, }; static struct vfsmount *smackfs_mount; -- cgit v1.2.3 From d297622875f9bedd8c1105e1797040e0b8d19402 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 21 Sep 2025 18:09:48 -0400 Subject: 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. Acked-by: Paul Moore Reviewed-by: Stephen Smalley Tested-by: Stephen Smalley Signed-off-by: Al Viro --- security/selinux/selinuxfs.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'security') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 232e087bce3e..b39e919c27b1 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, @@ -1871,23 +1869,24 @@ out: 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; @@ -2071,15 +2070,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; -- cgit v1.2.3 From d1e4a99358ea86854882ee325d4ceedd97f46e97 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 4 Oct 2025 16:54:10 -0400 Subject: selinuxfs: new helper for attaching files to tree allocating dentry after the inode has been set up reduces the amount of boilerplate - "attach this inode under that name and this parent or drop inode in case of failure" simplifies quite a few places. Acked-by: Paul Moore Reviewed-by: Stephen Smalley Tested-by: Stephen Smalley Signed-off-by: Al Viro --- security/selinux/selinuxfs.c | 160 ++++++++++++++++++------------------------- 1 file changed, 66 insertions(+), 94 deletions(-) (limited to 'security') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index b39e919c27b1..f088776dbbd3 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1197,6 +1197,25 @@ static struct inode *sel_make_inode(struct super_block *sb, umode_t mode) return ret; } +static struct dentry *sel_attach(struct dentry *parent, const char *name, + struct inode *inode) +{ + struct dentry *dentry = d_alloc_name(parent, name); + if (unlikely(!dentry)) { + iput(inode); + return ERR_PTR(-ENOMEM); + } + d_add(dentry, inode); + return dentry; +} + +static int sel_attach_file(struct dentry *parent, const char *name, + struct inode *inode) +{ + struct dentry *dentry = sel_attach(parent, name, inode); + return PTR_ERR_OR_ZERO(dentry); +} + static ssize_t sel_read_bool(struct file *filep, char __user *buf, size_t count, loff_t *ppos) { @@ -1356,8 +1375,7 @@ static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_ *bool_num = num; *bool_pending_names = names; - for (i = 0; i < num; i++) { - struct dentry *dentry; + for (i = 0; !ret && i < num; i++) { struct inode *inode; struct inode_security_struct *isec; ssize_t len; @@ -1368,15 +1386,9 @@ static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_ ret = -ENAMETOOLONG; break; } - dentry = d_alloc_name(bool_dir, names[i]); - if (!dentry) { - ret = -ENOMEM; - break; - } inode = sel_make_inode(bool_dir->d_sb, S_IFREG | S_IRUGO | S_IWUSR); if (!inode) { - dput(dentry); ret = -ENOMEM; break; } @@ -1394,7 +1406,8 @@ static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_ isec->initialized = LABEL_INITIALIZED; inode->i_fop = &sel_bool_ops; inode->i_ino = i|SEL_BOOL_INO_OFFSET; - d_add(dentry, inode); + + ret = sel_attach_file(bool_dir, names[i], inode); } out: free_page((unsigned long)page); @@ -1579,6 +1592,7 @@ static int sel_make_avc_files(struct dentry *dir) struct super_block *sb = dir->d_sb; struct selinux_fs_info *fsi = sb->s_fs_info; unsigned int i; + int err = 0; static const struct tree_descr files[] = { { "cache_threshold", &sel_avc_cache_threshold_ops, S_IRUGO|S_IWUSR }, @@ -1588,26 +1602,20 @@ static int sel_make_avc_files(struct dentry *dir) #endif }; - for (i = 0; i < ARRAY_SIZE(files); i++) { + for (i = 0; !err && i < ARRAY_SIZE(files); i++) { struct inode *inode; - struct dentry *dentry; - - dentry = d_alloc_name(dir, files[i].name); - if (!dentry) - return -ENOMEM; inode = sel_make_inode(dir->d_sb, S_IFREG|files[i].mode); - if (!inode) { - dput(dentry); + if (!inode) return -ENOMEM; - } inode->i_fop = files[i].ops; inode->i_ino = ++fsi->last_ino; - d_add(dentry, inode); + + err = sel_attach_file(dir, files[i].name, inode); } - return 0; + return err; } static int sel_make_ss_files(struct dentry *dir) @@ -1615,30 +1623,25 @@ static int sel_make_ss_files(struct dentry *dir) struct super_block *sb = dir->d_sb; struct selinux_fs_info *fsi = sb->s_fs_info; unsigned int i; + int err = 0; static const struct tree_descr files[] = { { "sidtab_hash_stats", &sel_sidtab_hash_stats_ops, S_IRUGO }, }; - for (i = 0; i < ARRAY_SIZE(files); i++) { + for (i = 0; !err && i < ARRAY_SIZE(files); i++) { struct inode *inode; - struct dentry *dentry; - - dentry = d_alloc_name(dir, files[i].name); - if (!dentry) - return -ENOMEM; inode = sel_make_inode(dir->d_sb, S_IFREG|files[i].mode); - if (!inode) { - dput(dentry); + if (!inode) return -ENOMEM; - } inode->i_fop = files[i].ops; inode->i_ino = ++fsi->last_ino; - d_add(dentry, inode); + + err = sel_attach_file(dir, files[i].name, inode); } - return 0; + return err; } static ssize_t sel_read_initcon(struct file *file, char __user *buf, @@ -1666,30 +1669,25 @@ static const struct file_operations sel_initcon_ops = { static int sel_make_initcon_files(struct dentry *dir) { unsigned int i; + int err = 0; - for (i = 1; i <= SECINITSID_NUM; i++) { - struct inode *inode; - struct dentry *dentry; + for (i = 1; !err && i <= SECINITSID_NUM; i++) { const char *s = security_get_initial_sid_context(i); + struct inode *inode; if (!s) continue; - dentry = d_alloc_name(dir, s); - if (!dentry) - return -ENOMEM; inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO); - if (!inode) { - dput(dentry); + if (!inode) return -ENOMEM; - } inode->i_fop = &sel_initcon_ops; inode->i_ino = i|SEL_INITCON_INO_OFFSET; - d_add(dentry, inode); + err = sel_attach_file(dir, s, inode); } - return 0; + return err; } static inline unsigned long sel_class_to_ino(u16 class) @@ -1771,29 +1769,21 @@ static int sel_make_perm_files(struct selinux_policy *newpolicy, if (rc) return rc; - for (i = 0; i < nperms; i++) { + for (i = 0; !rc && i < nperms; i++) { struct inode *inode; - struct dentry *dentry; - rc = -ENOMEM; - dentry = d_alloc_name(dir, perms[i]); - if (!dentry) - goto out; - - rc = -ENOMEM; inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO); if (!inode) { - dput(dentry); - goto out; + rc = -ENOMEM; + break; } inode->i_fop = &sel_perm_ops; /* i+1 since perm values are 1-indexed */ inode->i_ino = sel_perm_to_ino(classvalue, i + 1); - d_add(dentry, inode); + + rc = sel_attach_file(dir, perms[i], inode); } - rc = 0; -out: for (i = 0; i < nperms; i++) kfree(perms[i]); kfree(perms); @@ -1808,20 +1798,18 @@ static int sel_make_class_dir_entries(struct selinux_policy *newpolicy, struct selinux_fs_info *fsi = sb->s_fs_info; struct dentry *dentry = NULL; struct inode *inode = NULL; - - dentry = d_alloc_name(dir, "index"); - if (!dentry) - return -ENOMEM; + int err; inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO); - if (!inode) { - dput(dentry); + if (!inode) return -ENOMEM; - } inode->i_fop = &sel_class_ops; inode->i_ino = sel_class_to_ino(index); - d_add(dentry, inode); + + err = sel_attach_file(dir, "index", inode); + if (err) + return err; dentry = sel_make_dir(dir, "perms", &fsi->last_class_ino); if (IS_ERR(dentry)) @@ -1873,58 +1861,47 @@ 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; + int err = 0; + + for (iter = 0; !err && iter <= POLICYDB_CAP_MAX; iter++) { + const char *name; - for (iter = 0; iter <= POLICYDB_CAP_MAX; iter++) { if (iter < ARRAY_SIZE(selinux_policycap_names)) - dentry = d_alloc_name(dir, - selinux_policycap_names[iter]); + name = selinux_policycap_names[iter]; else - dentry = d_alloc_name(dir, "unknown"); - - if (dentry == NULL) - return -ENOMEM; + name = "unknown"; inode = sel_make_inode(sb, S_IFREG | 0444); - if (inode == NULL) { - dput(dentry); + if (!inode) return -ENOMEM; - } inode->i_fop = &sel_policycap_ops; inode->i_ino = iter | SEL_POLICYCAP_INO_OFFSET; - d_add(dentry, inode); + err = sel_attach_file(dir, name, inode); } - return 0; + return err; } static struct dentry *sel_make_dir(struct dentry *dir, const char *name, unsigned long *ino) { - struct dentry *dentry = d_alloc_name(dir, name); struct inode *inode; - if (!dentry) - return ERR_PTR(-ENOMEM); - inode = sel_make_inode(dir->d_sb, S_IFDIR | S_IRUGO | S_IXUGO); - if (!inode) { - dput(dentry); + if (!inode) return ERR_PTR(-ENOMEM); - } inode->i_op = &simple_dir_inode_operations; inode->i_fop = &simple_dir_operations; inode->i_ino = ++(*ino); /* directory inodes start off with i_nlink == 2 (for "." entry) */ inc_nlink(inode); - d_add(dentry, inode); /* bump link count on parent directory, too */ inc_nlink(d_inode(dir)); - return dentry; + return sel_attach(dir, name, inode); } static int reject_all(struct mnt_idmap *idmap, struct inode *inode, int mask) @@ -2012,17 +1989,10 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc) goto err; } - ret = -ENOMEM; - dentry = d_alloc_name(sb->s_root, NULL_FILE_NAME); - if (!dentry) - goto err; - ret = -ENOMEM; inode = sel_make_inode(sb, S_IFCHR | S_IRUGO | S_IWUGO); - if (!inode) { - dput(dentry); + if (!inode) goto err; - } inode->i_ino = ++fsi->last_ino; isec = selinux_inode(inode); @@ -2031,7 +2001,9 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc) isec->initialized = LABEL_INITIALIZED; init_special_inode(inode, S_IFCHR | S_IRUGO | S_IWUGO, MKDEV(MEM_MAJOR, 3)); - d_add(dentry, inode); + ret = sel_attach_file(sb->s_root, NULL_FILE_NAME, inode); + if (ret) + goto err; dentry = sel_make_dir(sb->s_root, "avc", &fsi->last_ino); if (IS_ERR(dentry)) { -- cgit v1.2.3 From cd08d17f39b7b7c54bfa35437fa4cd4e144d8179 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 26 Feb 2024 02:06:37 -0500 Subject: convert selinuxfs 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. Acked-by: Paul Moore Reviewed-by: Stephen Smalley Tested-by: Stephen Smalley Signed-off-by: Al Viro --- security/selinux/selinuxfs.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index f088776dbbd3..eae565358db4 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1205,7 +1205,8 @@ static struct dentry *sel_attach(struct dentry *parent, const char *name, iput(inode); return ERR_PTR(-ENOMEM); } - d_add(dentry, inode); + d_make_persistent(dentry, inode); + dput(dentry); return dentry; } @@ -1934,10 +1935,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 } #define NULL_FILE_NAME "null" @@ -2080,7 +2082,7 @@ static int sel_init_fs_context(struct fs_context *fc) static void sel_kill_sb(struct super_block *sb) { selinux_fs_info_free(sb); - kill_litter_super(sb); + kill_anon_super(sb); } static struct file_system_type sel_fs_type = { -- cgit v1.2.3 From 2026c6f8eb23df3b77b81d4d9a19d25126c2f1cf Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 14 May 2024 07:01:27 -0600 Subject: convert securityfs securityfs uses simple_recursive_removal(), but does not bother to mark dentries persistent. This is the only place where it still happens; get rid of that irregularity. * use simple_{start,done}_creating() and d_make_persitent(); kill_litter_super() use was already gone, since we empty the filesystem instance before it gets shut down. Acked-by: Paul Moore Signed-off-by: Al Viro --- security/inode.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) (limited to 'security') diff --git a/security/inode.c b/security/inode.c index bf7b5e2e6955..73df5db7f831 100644 --- a/security/inode.c +++ b/security/inode.c @@ -127,24 +127,19 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, parent = mount->mnt_root; } - dir = d_inode(parent); - - inode_lock(dir); - dentry = lookup_noperm(&QSTR(name), parent); - if (IS_ERR(dentry)) + inode = new_inode(parent->d_sb); + if (unlikely(!inode)) { + dentry = ERR_PTR(-ENOMEM); goto out; - - if (d_really_is_positive(dentry)) { - error = -EEXIST; - goto out1; } - inode = new_inode(dir->i_sb); - if (!inode) { - error = -ENOMEM; - goto out1; - } + dir = d_inode(parent); + dentry = simple_start_creating(parent, name); + if (IS_ERR(dentry)) { + iput(inode); + goto out; + } inode->i_ino = get_next_ino(); inode->i_mode = mode; simple_inode_init_ts(inode); @@ -160,15 +155,11 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, } else { inode->i_fop = fops; } - d_instantiate(dentry, inode); - inode_unlock(dir); - return dentry; + d_make_persistent(dentry, inode); + simple_done_creating(dentry); + return dentry; // borrowed -out1: - dput(dentry); - dentry = ERR_PTR(error); out: - inode_unlock(dir); if (pinned) simple_release_fs(&mount, &mount_count); return dentry; -- cgit v1.2.3 From eb028c33451af08bb34f45c6be6967ef1c98cbd1 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 27 Oct 2025 18:32:21 -0400 Subject: d_make_discardable(): warn if given a non-persistent dentry At this point there are very few call chains that might lead to d_make_discardable() on a dentry that hadn't been made persistent: calls of simple_unlink() and simple_rmdir() in configfs and apparmorfs. Both filesystems do pin (part of) their contents in dcache, but they are currently playing very unusual games with that. Converting them to more usual patterns might be possible, but it's definitely going to be a long series of changes in both cases. For now the easiest solution is to have both stop using simple_unlink() and simple_rmdir() - that allows to make d_make_discardable() warn when given a non-persistent dentry. Rather than giving them full-blown private copies (with calls of d_make_discardable() replaced with dput()), let's pull the parts of simple_unlink() and simple_rmdir() that deal with timestamps and link counts into separate helpers (__simple_unlink() and __simple_rmdir() resp.) and have those used by configfs and apparmorfs. Signed-off-by: Al Viro --- security/apparmor/apparmorfs.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 391a586d0557..9b9090d38ea2 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -358,10 +358,15 @@ static void aafs_remove(struct dentry *dentry) dir = d_inode(dentry->d_parent); inode_lock(dir); if (simple_positive(dentry)) { - if (d_is_dir(dentry)) - simple_rmdir(dir, dentry); - else - simple_unlink(dir, dentry); + if (d_is_dir(dentry)) { + if (!WARN_ON(!simple_empty(dentry))) { + __simple_rmdir(dir, dentry); + dput(dentry); + } + } else { + __simple_unlink(dir, dentry); + dput(dentry); + } d_delete(dentry); dput(dentry); } -- cgit v1.2.3