diff options
author | Linus Torvalds <torvalds@linux-foundation.org> | 2019-09-19 23:09:28 +0300 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2019-09-19 23:09:28 +0300 |
commit | c9fe5630dae1df2328d82042602e2c4d1add8d57 (patch) | |
tree | 7429c9a2f36fc8b1410cfebe45762cbc9b5d84b8 /fs/configfs | |
parent | 7e3d2c8210e67ebff472a0b371bb0efb4236ef52 (diff) | |
parent | e9c03af21cc7e5723d4f1e90fe45d2cdccb70dc7 (diff) | |
download | linux-c9fe5630dae1df2328d82042602e2c4d1add8d57.tar.xz |
Merge tag 'configfs-for-5.4' of git://git.infradead.org/users/hch/configfs
Pull configfs updates from Christoph Hellwig:
- fix a symlink deadlock (Al Viro)
- various cleanups (Al Viro, me)
* tag 'configfs-for-5.4' of git://git.infradead.org/users/hch/configfs:
configfs: calculate the symlink target only once
configfs: make configfs_create() return inode
configfs: factor dirent removal into helpers
configfs: fix a deadlock in configfs_symlink()
Diffstat (limited to 'fs/configfs')
-rw-r--r-- | fs/configfs/configfs_internal.h | 21 | ||||
-rw-r--r-- | fs/configfs/dir.c | 173 | ||||
-rw-r--r-- | fs/configfs/inode.c | 24 | ||||
-rw-r--r-- | fs/configfs/mount.c | 9 | ||||
-rw-r--r-- | fs/configfs/symlink.c | 197 |
5 files changed, 170 insertions, 254 deletions
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h index 520f1813e789..22dce2d35a4b 100644 --- a/fs/configfs/configfs_internal.h +++ b/fs/configfs/configfs_internal.h @@ -34,7 +34,7 @@ struct configfs_dirent { int s_dependent_count; struct list_head s_sibling; struct list_head s_children; - struct list_head s_links; + int s_links; void * s_element; int s_type; umode_t s_mode; @@ -66,7 +66,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 *, @@ -84,7 +84,6 @@ extern int configfs_setattr(struct dentry *dentry, struct iattr *iattr); extern struct dentry *configfs_pin_fs(void); extern void configfs_release_fs(void); -extern struct rw_semaphore configfs_rename_sem; extern const struct file_operations configfs_dir_operations; extern const struct file_operations configfs_file_operations; extern const struct file_operations configfs_bin_file_operations; @@ -97,14 +96,8 @@ extern int configfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname); extern int configfs_unlink(struct inode *dir, struct dentry *dentry); -struct configfs_symlink { - struct list_head sl_list; - struct config_item *sl_target; -}; - -extern int configfs_create_link(struct configfs_symlink *sl, - struct dentry *parent, - struct dentry *dentry); +int configfs_create_link(struct configfs_dirent *target, struct dentry *parent, + struct dentry *dentry, char *body); static inline struct config_item * to_item(struct dentry * dentry) { @@ -132,11 +125,7 @@ static inline struct config_item *configfs_get_config_item(struct dentry *dentry spin_lock(&dentry->d_lock); if (!d_unhashed(dentry)) { struct configfs_dirent * sd = dentry->d_fsdata; - if (sd->s_type & CONFIGFS_ITEM_LINK) { - struct configfs_symlink * sl = sd->s_element; - item = config_item_get(sl->sl_target); - } else - item = config_item_get(sd->s_element); + item = config_item_get(sd->s_element); } spin_unlock(&dentry->d_lock); diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 79fc25aaa8cd..cf7b7e1d5bd7 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -22,7 +22,6 @@ #include <linux/configfs.h> #include "configfs_internal.h" -DECLARE_RWSEM(configfs_rename_sem); /* * Protects mutations of configfs_dirent linkage together with proper i_mutex * Also protects mutations of symlinks linkage to target configfs_dirent @@ -191,7 +190,6 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *paren return ERR_PTR(-ENOMEM); atomic_set(&sd->s_count, 1); - INIT_LIST_HEAD(&sd->s_links); INIT_LIST_HEAD(&sd->s_children); sd->s_element = element; sd->s_type = type; @@ -253,30 +251,16 @@ int configfs_make_dirent(struct configfs_dirent * parent_sd, return 0; } -static void init_dir(struct inode * inode) +static void configfs_remove_dirent(struct dentry *dentry) { - 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; -} + struct configfs_dirent *sd = dentry->d_fsdata; -static void init_symlink(struct inode * inode) -{ - inode->i_op = &configfs_symlink_inode_operations; + if (!sd) + return; + spin_lock(&configfs_dirent_lock); + list_del_init(&sd->s_sibling); + spin_unlock(&configfs_dirent_lock); + configfs_put(sd); } /** @@ -294,6 +278,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); @@ -308,20 +293,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) { - 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); - } - } - return 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 PTR_ERR(inode); } /* @@ -362,41 +351,40 @@ int configfs_dirent_is_ready(struct configfs_dirent *sd) return ret; } -int configfs_create_link(struct configfs_symlink *sl, - struct dentry *parent, - struct dentry *dentry) +int configfs_create_link(struct configfs_dirent *target, struct dentry *parent, + struct dentry *dentry, char *body) { 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) { - 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); - } - } - } - return err; + err = configfs_make_dirent(p, dentry, target, mode, CONFIGFS_ITEM_LINK, + p->s_frag); + if (err) + return err; + + inode = configfs_create(dentry, mode); + if (IS_ERR(inode)) + goto out_remove; + + inode->i_link = body; + 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 PTR_ERR(inode); } 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); @@ -437,20 +425,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, @@ -520,7 +515,7 @@ static int configfs_detach_prep(struct dentry *dentry, struct dentry **wait) parent_sd->s_type |= CONFIGFS_USET_DROPPING; ret = -EBUSY; - if (!list_empty(&parent_sd->s_links)) + if (parent_sd->s_links) goto out; ret = 0; @@ -1578,44 +1573,6 @@ const struct inode_operations configfs_root_inode_operations = { .setattr = configfs_setattr, }; -#if 0 -int configfs_rename_dir(struct config_item * item, const char *new_name) -{ - int error = 0; - struct dentry * new_dentry, * parent; - - if (!strcmp(config_item_name(item), new_name)) - return -EINVAL; - - if (!item->parent) - return -EINVAL; - - down_write(&configfs_rename_sem); - parent = item->parent->dentry; - - inode_lock(d_inode(parent)); - - new_dentry = lookup_one_len(new_name, parent, strlen(new_name)); - if (!IS_ERR(new_dentry)) { - if (d_really_is_negative(new_dentry)) { - error = config_item_set_name(item, "%s", new_name); - if (!error) { - d_add(new_dentry, NULL); - d_move(item->dentry, new_dentry); - } - else - d_delete(new_dentry); - } else - error = -EEXIST; - dput(new_dentry); - } - inode_unlock(d_inode(parent)); - up_write(&configfs_rename_sem); - - return error; -} -#endif - static int configfs_dir_open(struct inode *inode, struct file *file) { struct dentry * dentry = file->f_path.dentry; diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c index 884dcf06cfbe..680aba9c00d5 100644 --- a/fs/configfs/inode.c +++ b/fs/configfs/inode.c @@ -164,41 +164,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; } /* diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c index 55438dd58189..0c6e8cf61953 100644 --- a/fs/configfs/mount.c +++ b/fs/configfs/mount.c @@ -28,9 +28,18 @@ static struct vfsmount *configfs_mount = NULL; struct kmem_cache *configfs_dir_cachep; static int configfs_mnt_count = 0; + +static void configfs_free_inode(struct inode *inode) +{ + if (S_ISLNK(inode->i_mode)) + kfree(inode->i_link); + free_inode_nonrcu(inode); +} + static const struct super_operations configfs_ops = { .statfs = simple_statfs, .drop_inode = generic_delete_inode, + .free_inode = configfs_free_inode, }; static struct config_group configfs_root_group = { diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c index 91eac6c55e07..dc5dbf6a81d7 100644 --- a/fs/configfs/symlink.c +++ b/fs/configfs/symlink.c @@ -55,41 +55,63 @@ static void fill_item_path(struct config_item * item, char * buffer, int length) } } +static int configfs_get_target_path(struct config_item *item, + struct config_item *target, char *path) +{ + int depth, size; + char *s; + + depth = item_depth(item); + size = item_path_length(target) + depth * 3 - 1; + if (size > PATH_MAX) + return -ENAMETOOLONG; + + pr_debug("%s: depth = %d, size = %d\n", __func__, depth, size); + + for (s = path; depth--; s += 3) + strcpy(s,"../"); + + fill_item_path(target, path, size); + pr_debug("%s: path = '%s'\n", __func__, path); + return 0; +} + static int create_link(struct config_item *parent_item, struct config_item *item, struct dentry *dentry) { struct configfs_dirent *target_sd = item->ci_dentry->d_fsdata; - struct configfs_symlink *sl; + char *body; int ret; - ret = -ENOENT; if (!configfs_dirent_is_ready(target_sd)) - goto out; - ret = -ENOMEM; - sl = kmalloc(sizeof(struct configfs_symlink), GFP_KERNEL); - if (sl) { + return -ENOENT; + + body = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!body) + return -ENOMEM; + + configfs_get(target_sd); + spin_lock(&configfs_dirent_lock); + if (target_sd->s_type & CONFIGFS_USET_DROPPING) { + spin_unlock(&configfs_dirent_lock); + configfs_put(target_sd); + kfree(body); + return -ENOENT; + } + target_sd->s_links++; + spin_unlock(&configfs_dirent_lock); + ret = configfs_get_target_path(item, item, body); + if (!ret) + ret = configfs_create_link(target_sd, parent_item->ci_dentry, + dentry, body); + if (ret) { spin_lock(&configfs_dirent_lock); - if (target_sd->s_type & CONFIGFS_USET_DROPPING) { - spin_unlock(&configfs_dirent_lock); - kfree(sl); - return -ENOENT; - } - sl->sl_target = config_item_get(item); - list_add(&sl->sl_list, &target_sd->s_links); + target_sd->s_links--; spin_unlock(&configfs_dirent_lock); - ret = configfs_create_link(sl, parent_item->ci_dentry, - dentry); - if (ret) { - spin_lock(&configfs_dirent_lock); - list_del_init(&sl->sl_list); - spin_unlock(&configfs_dirent_lock); - config_item_put(item); - kfree(sl); - } + configfs_put(target_sd); + kfree(body); } - -out: return ret; } @@ -131,9 +153,8 @@ int configfs_symlink(struct inode *dir, struct dentry *dentry, const char *symna * Fake invisibility if dir belongs to a group/default groups hierarchy * being attached */ - ret = -ENOENT; if (!configfs_dirent_is_ready(sd)) - goto out; + return -ENOENT; parent_item = configfs_get_config_item(dentry->d_parent); type = parent_item->ci_type; @@ -143,11 +164,42 @@ int configfs_symlink(struct inode *dir, struct dentry *dentry, const char *symna !type->ct_item_ops->allow_link) goto out_put; + /* + * This is really sick. What they wanted was a hybrid of + * link(2) and symlink(2) - they wanted the target resolved + * at syscall time (as link(2) would've done), be a directory + * (which link(2) would've refused to do) *AND* be a deep + * fucking magic, making the target busy from rmdir POV. + * symlink(2) is nothing of that sort, and the locking it + * gets matches the normal symlink(2) semantics. Without + * attempts to resolve the target (which might very well + * not even exist yet) done prior to locking the parent + * directory. This perversion, OTOH, needs to resolve + * the target, which would lead to obvious deadlocks if + * attempted with any directories locked. + * + * Unfortunately, that garbage is userland ABI and we should've + * said "no" back in 2005. Too late now, so we get to + * play very ugly games with locking. + * + * Try *ANYTHING* of that sort in new code, and you will + * really regret it. Just ask yourself - what could a BOFH + * do to me and do I want to find it out first-hand? + * + * AV, a thoroughly annoyed bastard. + */ + inode_unlock(dir); ret = get_target(symname, &path, &target_item, dentry->d_sb); + inode_lock(dir); if (ret) goto out_put; - ret = type->ct_item_ops->allow_link(parent_item, target_item); + if (dentry->d_inode || d_unhashed(dentry)) + ret = -EEXIST; + else + ret = inode_permission(dir, MAY_WRITE | MAY_EXEC); + if (!ret) + ret = type->ct_item_ops->allow_link(parent_item, target_item); if (!ret) { mutex_lock(&configfs_symlink_mutex); ret = create_link(parent_item, target_item, dentry); @@ -162,15 +214,12 @@ int configfs_symlink(struct inode *dir, struct dentry *dentry, const char *symna out_put: config_item_put(parent_item); - -out: return ret; } int configfs_unlink(struct inode *dir, struct dentry *dentry) { - struct configfs_dirent *sd = dentry->d_fsdata; - struct configfs_symlink *sl; + struct configfs_dirent *sd = dentry->d_fsdata, *target_sd; struct config_item *parent_item; const struct config_item_type *type; int ret; @@ -179,7 +228,7 @@ int configfs_unlink(struct inode *dir, struct dentry *dentry) if (!(sd->s_type & CONFIGFS_ITEM_LINK)) goto out; - sl = sd->s_element; + target_sd = sd->s_element; parent_item = configfs_get_config_item(dentry->d_parent); type = parent_item->ci_type; @@ -193,21 +242,18 @@ int configfs_unlink(struct inode *dir, struct dentry *dentry) /* * drop_link() must be called before - * list_del_init(&sl->sl_list), so that the order of + * decrementing target's ->s_links, so that the order of * drop_link(this, target) and drop_item(target) is preserved. */ if (type && type->ct_item_ops && type->ct_item_ops->drop_link) type->ct_item_ops->drop_link(parent_item, - sl->sl_target); + target_sd->s_element); spin_lock(&configfs_dirent_lock); - list_del_init(&sl->sl_list); + target_sd->s_links--; spin_unlock(&configfs_dirent_lock); - - /* Put reference from create_link() */ - config_item_put(sl->sl_target); - kfree(sl); + configfs_put(target_sd); config_item_put(parent_item); @@ -217,79 +263,8 @@ out: return ret; } -static int configfs_get_target_path(struct config_item * item, struct config_item * target, - char *path) -{ - char * s; - int depth, size; - - depth = item_depth(item); - size = item_path_length(target) + depth * 3 - 1; - if (size > PATH_MAX) - return -ENAMETOOLONG; - - pr_debug("%s: depth = %d, size = %d\n", __func__, depth, size); - - for (s = path; depth--; s += 3) - strcpy(s,"../"); - - fill_item_path(target, path, size); - pr_debug("%s: path = '%s'\n", __func__, path); - - return 0; -} - -static int configfs_getlink(struct dentry *dentry, char * path) -{ - struct config_item *item, *target_item; - int error = 0; - - item = configfs_get_config_item(dentry->d_parent); - if (!item) - return -EINVAL; - - target_item = configfs_get_config_item(dentry); - if (!target_item) { - config_item_put(item); - return -EINVAL; - } - - down_read(&configfs_rename_sem); - error = configfs_get_target_path(item, target_item, path); - up_read(&configfs_rename_sem); - - config_item_put(item); - config_item_put(target_item); - return error; - -} - -static const char *configfs_get_link(struct dentry *dentry, - struct inode *inode, - struct delayed_call *done) -{ - char *body; - int error; - - if (!dentry) - return ERR_PTR(-ECHILD); - - body = kzalloc(PAGE_SIZE, GFP_KERNEL); - if (!body) - return ERR_PTR(-ENOMEM); - - error = configfs_getlink(dentry, body); - if (!error) { - set_delayed_call(done, kfree_link, body); - return body; - } - - kfree(body); - return ERR_PTR(error); -} - const struct inode_operations configfs_symlink_inode_operations = { - .get_link = configfs_get_link, + .get_link = simple_get_link, .setattr = configfs_setattr, }; |