On Tue, Oct 28, 2025 at 3:56 AM Al Viro <[email protected]> wrote: > > 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. > > Signed-off-by: Al Viro <[email protected]>
Reviewed-by: Stephen Smalley <[email protected]> Tested-by: Stephen Smalley <[email protected]> > --- > security/selinux/selinuxfs.c | 160 +++++++++++++++-------------------- > 1 file changed, 66 insertions(+), 94 deletions(-) > > 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)) { > -- > 2.47.3 > >
