summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLinus Torvalds <torvalds@linux-foundation.org>2019-09-19 13:09:28 -0700
committerLinus Torvalds <torvalds@linux-foundation.org>2019-09-19 13:09:28 -0700
commitc9fe5630dae1df2328d82042602e2c4d1add8d57 (patch)
tree7429c9a2f36fc8b1410cfebe45762cbc9b5d84b8
parent7e3d2c8210e67ebff472a0b371bb0efb4236ef52 (diff)
parente9c03af21cc7e5723d4f1e90fe45d2cdccb70dc7 (diff)
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()
-rw-r--r--fs/configfs/configfs_internal.h21
-rw-r--r--fs/configfs/dir.c173
-rw-r--r--fs/configfs/inode.c24
-rw-r--r--fs/configfs/mount.c9
-rw-r--r--fs/configfs/symlink.c197
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,
};