Commit c9fe5630 authored by Linus Torvalds's avatar Linus Torvalds
Browse files

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()
parents 7e3d2c82 e9c03af2
Loading
Loading
Loading
Loading
+5 −16
Original line number Diff line number Diff line
@@ -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,10 +125,6 @@ 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);
	}
	spin_unlock(&dentry->d_lock);
+65 −108
Original line number Diff line number Diff line
@@ -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) {
	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;
	} 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;
	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);
			}
		}
	}
	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;
+5 −19
Original line number Diff line number Diff line
@@ -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;
}

/*
+9 −0
Original line number Diff line number Diff line
@@ -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 = {
+86 −111
Original line number Diff line number Diff line
@@ -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);
			kfree(sl);
		configfs_put(target_sd);
		kfree(body);
		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);
	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);
			list_del_init(&sl->sl_list);
		target_sd->s_links--;
		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,10 +164,41 @@ 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;

	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);
@@ -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,
};