[PATCH openEuler-1.0-LTS 1/6] configfs: return -ENAMETOOLONG earlier in configfs_lookup
From: Christoph Hellwig <hch@lst.de> mainline inclusion from mainline-v5.15-rc1 commit 417b962ddeca2b70eb72d28c87541bdad4e234f8 category: bugfix bugzilla: 187567, https://gitee.com/openeuler/kernel/issues/I5PK1G CVE: NA -------------------------------- Just like most other file systems: get the simple checks out of the way first. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- fs/configfs/dir.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 9b43611e1bb3..ff298a9baa0a 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -483,6 +483,9 @@ static struct dentry * configfs_lookup(struct inode *dir, int found = 0; int err; + if (dentry->d_name.len > NAME_MAX) + return ERR_PTR(-ENAMETOOLONG); + /* * Fake invisibility if dir belongs to a group/default groups hierarchy * being attached @@ -513,8 +516,6 @@ static struct dentry * configfs_lookup(struct inode *dir, * If it doesn't exist and it isn't a NOT_PINNED item, * it must be negative. */ - if (dentry->d_name.len > NAME_MAX) - return ERR_PTR(-ENAMETOOLONG); d_add(dentry, NULL); return NULL; } -- 2.25.1
From: Christoph Hellwig <hch@lst.de> mainline inclusion from mainline-v5.15-rc1 commit 899587c8d0908e5124fd074d52bf05b4b0633a79 category: bugfix bugzilla: 187567, https://gitee.com/openeuler/kernel/issues/I5PK1G CVE: NA -------------------------------- Return the error directly instead of using a goto. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- fs/configfs/dir.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index ff298a9baa0a..26bd2004fe35 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -494,9 +494,8 @@ static struct dentry * configfs_lookup(struct inode *dir, * not complete their initialization, since the dentries of the * attributes won't be instantiated. */ - err = -ENOENT; if (!configfs_dirent_is_ready(parent_sd)) - goto out; + return ERR_PTR(-ENOENT); list_for_each_entry(sd, &parent_sd->s_children, s_sibling) { if (sd->s_type & CONFIGFS_NOT_PINNED) { @@ -520,7 +519,6 @@ static struct dentry * configfs_lookup(struct inode *dir, return NULL; } -out: return ERR_PTR(err); } -- 2.25.1
From: Christoph Hellwig <hch@lst.de> mainline inclusion from mainline-v5.4-rc1 commit 1cf7a003b044744c06dfa452cd136e71223b5569 category: bugfix bugzilla: 187567, https://gitee.com/openeuler/kernel/issues/I5PK1G CVE: NA -------------------------------- Lots of duplicated code that benefits from a little consolidation. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- fs/configfs/dir.c | 63 +++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 26bd2004fe35..cbec81662a2a 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -274,6 +274,18 @@ int configfs_make_dirent(struct configfs_dirent * parent_sd, return 0; } +static void configfs_remove_dirent(struct dentry *dentry) +{ + struct configfs_dirent *sd = dentry->d_fsdata; + + if (!sd) + return; + spin_lock(&configfs_dirent_lock); + list_del_init(&sd->s_sibling); + spin_unlock(&configfs_dirent_lock); + configfs_put(sd); +} + static void init_dir(struct inode * inode) { inode->i_op = &configfs_dir_inode_operations; @@ -330,18 +342,15 @@ static int configfs_create_dir(struct config_item *item, struct dentry *dentry, configfs_set_dir_dirent_depth(p->d_fsdata, dentry->d_fsdata); error = configfs_create(dentry, mode, init_dir); - if (!error) { - inc_nlink(d_inode(p)); - item->ci_dentry = dentry; - } else { - struct configfs_dirent *sd = dentry->d_fsdata; - if (sd) { - spin_lock(&configfs_dirent_lock); - list_del_init(&sd->s_sibling); - spin_unlock(&configfs_dirent_lock); - configfs_put(sd); - } - } + if (error) + goto out_remove; + + inc_nlink(d_inode(p)); + item->ci_dentry = dentry; + return 0; + +out_remove: + configfs_remove_dirent(dentry); return error; } @@ -393,31 +402,25 @@ int configfs_create_link(struct configfs_symlink *sl, err = configfs_make_dirent(p, dentry, sl, mode, CONFIGFS_ITEM_LINK, p->s_frag); - if (!err) { - err = configfs_create(dentry, mode, init_symlink); - if (err) { - struct configfs_dirent *sd = dentry->d_fsdata; - if (sd) { - spin_lock(&configfs_dirent_lock); - list_del_init(&sd->s_sibling); - spin_unlock(&configfs_dirent_lock); - configfs_put(sd); - } - } - } + if (err) + return err; + + err = configfs_create(dentry, mode, init_symlink); + if (err) + goto out_remove; + return 0; + +out_remove: + configfs_remove_dirent(dentry); return err; } static void remove_dir(struct dentry * d) { struct dentry * parent = dget(d->d_parent); - struct configfs_dirent * sd; - sd = d->d_fsdata; - spin_lock(&configfs_dirent_lock); - list_del_init(&sd->s_sibling); - spin_unlock(&configfs_dirent_lock); - configfs_put(sd); + configfs_remove_dirent(d); + if (d_really_is_positive(d)) simple_rmdir(d_inode(parent),d); -- 2.25.1
From: Al Viro <viro@zeniv.linux.org.uk> mainline inclusion from mainline-v5.4-rc1 commit 2743c515a1239bb96028bddafef87c0a486f4361 category: bugfix bugzilla: 187567, https://gitee.com/openeuler/kernel/issues/I5PK1G CVE: NA -------------------------------- Get rid of the callback, deal with that and dentry in callers Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- fs/configfs/configfs_internal.h | 2 +- fs/configfs/dir.c | 72 +++++++++++++++------------------ fs/configfs/inode.c | 24 +++-------- 3 files changed, 39 insertions(+), 59 deletions(-) diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h index 16eb59adf5aa..e27f7e475d8d 100644 --- a/fs/configfs/configfs_internal.h +++ b/fs/configfs/configfs_internal.h @@ -80,7 +80,7 @@ extern struct kmem_cache *configfs_dir_cachep; extern int configfs_is_root(struct config_item *item); extern struct inode * configfs_new_inode(umode_t mode, struct configfs_dirent *, struct super_block *); -extern int configfs_create(struct dentry *, umode_t mode, void (*init)(struct inode *)); +extern struct inode *configfs_create(struct dentry *, umode_t mode); extern int configfs_create_file(struct config_item *, const struct configfs_attribute *); extern int configfs_create_bin_file(struct config_item *, diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index cbec81662a2a..53acfef733cf 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -286,32 +286,6 @@ static void configfs_remove_dirent(struct dentry *dentry) configfs_put(sd); } -static void init_dir(struct inode * inode) -{ - inode->i_op = &configfs_dir_inode_operations; - inode->i_fop = &configfs_dir_operations; - - /* directory inodes start off with i_nlink == 2 (for "." entry) */ - inc_nlink(inode); -} - -static void configfs_init_file(struct inode * inode) -{ - inode->i_size = PAGE_SIZE; - inode->i_fop = &configfs_file_operations; -} - -static void configfs_init_bin_file(struct inode *inode) -{ - inode->i_size = 0; - inode->i_fop = &configfs_bin_file_operations; -} - -static void init_symlink(struct inode * inode) -{ - inode->i_op = &configfs_symlink_inode_operations; -} - /** * configfs_create_dir - create a directory for an config_item. * @item: config_itemwe're creating directory for. @@ -327,6 +301,7 @@ static int configfs_create_dir(struct config_item *item, struct dentry *dentry, int error; umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO; struct dentry *p = dentry->d_parent; + struct inode *inode; BUG_ON(!item); @@ -341,17 +316,24 @@ static int configfs_create_dir(struct config_item *item, struct dentry *dentry, return error; configfs_set_dir_dirent_depth(p->d_fsdata, dentry->d_fsdata); - error = configfs_create(dentry, mode, init_dir); - if (error) + inode = configfs_create(dentry, mode); + if (IS_ERR(inode)) goto out_remove; + inode->i_op = &configfs_dir_inode_operations; + inode->i_fop = &configfs_dir_operations; + /* directory inodes start off with i_nlink == 2 (for "." entry) */ + inc_nlink(inode); + d_instantiate(dentry, inode); + /* already hashed */ + dget(dentry); /* pin directory dentries in core */ inc_nlink(d_inode(p)); item->ci_dentry = dentry; return 0; out_remove: configfs_remove_dirent(dentry); - return error; + return PTR_ERR(inode); } /* @@ -399,20 +381,25 @@ int configfs_create_link(struct configfs_symlink *sl, int err = 0; umode_t mode = S_IFLNK | S_IRWXUGO; struct configfs_dirent *p = parent->d_fsdata; + struct inode *inode; err = configfs_make_dirent(p, dentry, sl, mode, CONFIGFS_ITEM_LINK, p->s_frag); if (err) return err; - err = configfs_create(dentry, mode, init_symlink); - if (err) + inode = configfs_create(dentry, mode); + if (IS_ERR(inode)) goto out_remove; + + inode->i_op = &configfs_symlink_inode_operations; + d_instantiate(dentry, inode); + dget(dentry); /* pin link dentries in core */ return 0; out_remove: configfs_remove_dirent(dentry); - return err; + return PTR_ERR(inode); } static void remove_dir(struct dentry * d) @@ -461,20 +448,27 @@ static void configfs_remove_dir(struct config_item * item) static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * dentry) { struct configfs_attribute * attr = sd->s_element; - int error; + struct inode *inode; spin_lock(&configfs_dirent_lock); dentry->d_fsdata = configfs_get(sd); sd->s_dentry = dentry; spin_unlock(&configfs_dirent_lock); - error = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG, - (sd->s_type & CONFIGFS_ITEM_BIN_ATTR) ? - configfs_init_bin_file : - configfs_init_file); - if (error) + inode = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG); + if (IS_ERR(inode)) { configfs_put(sd); - return error; + return PTR_ERR(inode); + } + if (sd->s_type & CONFIGFS_ITEM_BIN_ATTR) { + inode->i_size = 0; + inode->i_fop = &configfs_bin_file_operations; + } else { + inode->i_size = PAGE_SIZE; + inode->i_fop = &configfs_file_operations; + } + d_add(dentry, inode); + return 0; } static struct dentry * configfs_lookup(struct inode *dir, diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c index 28ef9e528853..6658e2cddcd8 100644 --- a/fs/configfs/inode.c +++ b/fs/configfs/inode.c @@ -178,41 +178,27 @@ static void configfs_set_inode_lock_class(struct configfs_dirent *sd, #endif /* CONFIG_LOCKDEP */ -int configfs_create(struct dentry * dentry, umode_t mode, void (*init)(struct inode *)) +struct inode *configfs_create(struct dentry *dentry, umode_t mode) { - int error = 0; struct inode *inode = NULL; struct configfs_dirent *sd; struct inode *p_inode; if (!dentry) - return -ENOENT; + return ERR_PTR(-ENOENT); if (d_really_is_positive(dentry)) - return -EEXIST; + return ERR_PTR(-EEXIST); sd = dentry->d_fsdata; inode = configfs_new_inode(mode, sd, dentry->d_sb); if (!inode) - return -ENOMEM; + return ERR_PTR(-ENOMEM); p_inode = d_inode(dentry->d_parent); p_inode->i_mtime = p_inode->i_ctime = current_time(p_inode); configfs_set_inode_lock_class(sd, inode); - - init(inode); - if (S_ISDIR(mode) || S_ISLNK(mode)) { - /* - * ->symlink(), ->mkdir(), configfs_register_subsystem() or - * create_default_group() - already hashed. - */ - d_instantiate(dentry, inode); - dget(dentry); /* pin link and directory dentries in core */ - } else { - /* ->lookup() */ - d_add(dentry, inode); - } - return error; + return inode; } /* -- 2.25.1
From: Christoph Hellwig <hch@lst.de> mainline inclusion from mainline-v5.15-rc1 commit d07f132a225c013e59aa77f514ad9211ecab82ee category: bugfix bugzilla: 187567, https://gitee.com/openeuler/kernel/issues/I5PK1G CVE: NA -------------------------------- This makes it more clear what gets added to the dcache and prepares for an additional locking fix. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- fs/configfs/dir.c | 73 ++++++++++++++++------------------------------- 1 file changed, 24 insertions(+), 49 deletions(-) diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 53acfef733cf..b4e742acfdcf 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -69,7 +69,7 @@ static void configfs_d_iput(struct dentry * dentry, /* * Set sd->s_dentry to null only when this dentry is the one * that is going to be killed. Otherwise configfs_d_iput may - * run just after configfs_attach_attr and set sd->s_dentry to + * run just after configfs_lookup and set sd->s_dentry to * NULL even it's still in use. */ if (sd->s_dentry == dentry) @@ -441,44 +441,13 @@ static void configfs_remove_dir(struct config_item * item) dput(dentry); } - -/* attaches attribute's configfs_dirent to the dentry corresponding to the - * attribute file - */ -static int configfs_attach_attr(struct configfs_dirent * sd, struct dentry * dentry) -{ - struct configfs_attribute * attr = sd->s_element; - struct inode *inode; - - spin_lock(&configfs_dirent_lock); - dentry->d_fsdata = configfs_get(sd); - sd->s_dentry = dentry; - spin_unlock(&configfs_dirent_lock); - - inode = configfs_create(dentry, (attr->ca_mode & S_IALLUGO) | S_IFREG); - if (IS_ERR(inode)) { - configfs_put(sd); - return PTR_ERR(inode); - } - if (sd->s_type & CONFIGFS_ITEM_BIN_ATTR) { - inode->i_size = 0; - inode->i_fop = &configfs_bin_file_operations; - } else { - inode->i_size = PAGE_SIZE; - inode->i_fop = &configfs_file_operations; - } - d_add(dentry, inode); - return 0; -} - static struct dentry * configfs_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags) { struct configfs_dirent * parent_sd = dentry->d_parent->d_fsdata; struct configfs_dirent * sd; - int found = 0; - int err; + struct inode *inode = NULL; if (dentry->d_name.len > NAME_MAX) return ERR_PTR(-ENAMETOOLONG); @@ -495,28 +464,34 @@ static struct dentry * configfs_lookup(struct inode *dir, return ERR_PTR(-ENOENT); list_for_each_entry(sd, &parent_sd->s_children, s_sibling) { - if (sd->s_type & CONFIGFS_NOT_PINNED) { - const unsigned char * name = configfs_get_name(sd); + if ((sd->s_type & CONFIGFS_NOT_PINNED) && + !strcmp(configfs_get_name(sd), dentry->d_name.name)) { + struct configfs_attribute *attr = sd->s_element; + umode_t mode = (attr->ca_mode & S_IALLUGO) | S_IFREG; - if (strcmp(name, dentry->d_name.name)) - continue; + spin_lock(&configfs_dirent_lock); + dentry->d_fsdata = configfs_get(sd); + sd->s_dentry = dentry; + spin_unlock(&configfs_dirent_lock); - found = 1; - err = configfs_attach_attr(sd, dentry); + inode = configfs_create(dentry, mode); + if (IS_ERR(inode)) { + configfs_put(sd); + return ERR_CAST(inode); + } + if (sd->s_type & CONFIGFS_ITEM_BIN_ATTR) { + inode->i_size = 0; + inode->i_fop = &configfs_bin_file_operations; + } else { + inode->i_size = PAGE_SIZE; + inode->i_fop = &configfs_file_operations; + } break; } } - if (!found) { - /* - * If it doesn't exist and it isn't a NOT_PINNED item, - * it must be negative. - */ - d_add(dentry, NULL); - return NULL; - } - - return ERR_PTR(err); + d_add(dentry, inode); + return NULL; } /* -- 2.25.1
From: Sishuai Gong <sishuai@purdue.edu> mainline inclusion from mainline-v5.15-rc1 commit c42dd069be8dfc9b2239a5c89e73bbd08ab35de0 category: bugfix bugzilla: 187567, https://gitee.com/openeuler/kernel/issues/I5PK1G CVE: NA -------------------------------- When configfs_lookup() is executing list_for_each_entry(), it is possible that configfs_dir_lseek() is calling list_del(). Some unfortunate interleavings of them can cause a kernel NULL pointer dereference error Thread 1 Thread 2 //configfs_dir_lseek() //configfs_lookup() list_del(&cursor->s_sibling); list_for_each_entry(sd, ...) Fix this by grabbing configfs_dirent_lock in configfs_lookup() while iterating ->s_children. Signed-off-by: Sishuai Gong <sishuai@purdue.edu> Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com> --- fs/configfs/dir.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index b4e742acfdcf..687735be3cd4 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -463,13 +463,13 @@ static struct dentry * configfs_lookup(struct inode *dir, if (!configfs_dirent_is_ready(parent_sd)) return ERR_PTR(-ENOENT); + spin_lock(&configfs_dirent_lock); list_for_each_entry(sd, &parent_sd->s_children, s_sibling) { if ((sd->s_type & CONFIGFS_NOT_PINNED) && !strcmp(configfs_get_name(sd), dentry->d_name.name)) { struct configfs_attribute *attr = sd->s_element; umode_t mode = (attr->ca_mode & S_IALLUGO) | S_IFREG; - spin_lock(&configfs_dirent_lock); dentry->d_fsdata = configfs_get(sd); sd->s_dentry = dentry; spin_unlock(&configfs_dirent_lock); @@ -486,10 +486,11 @@ static struct dentry * configfs_lookup(struct inode *dir, inode->i_size = PAGE_SIZE; inode->i_fop = &configfs_file_operations; } - break; + goto done; } } - + spin_unlock(&configfs_dirent_lock); +done: d_add(dentry, inode); return NULL; } -- 2.25.1
participants (1)
-
Yongqiang Liu