Commit eed7a0db authored by Joel Becker's avatar Joel Becker Committed by Mark Fasheh

configfs: configfs_mkdir() failed to cleanup linkage.

If configfs_mkdir() errored in certain ways after the parent<->child
linkage was already created, it would not undo the linkage.  Also,
comment the reference counting for clarity.
Signed-off-by: default avatarJoel Becker <joel.becker@oracle.com>
Signed-off-by: default avatarMark Fasheh <mark.fasheh@oracle.com>
parent 84efad1a
......@@ -505,13 +505,15 @@ static int populate_groups(struct config_group *group)
int i;
if (group->default_groups) {
/* FYI, we're faking mkdir here
/*
* FYI, we're faking mkdir here
* I'm not sure we need this semaphore, as we're called
* from our parent's mkdir. That holds our parent's
* i_mutex, so afaik lookup cannot continue through our
* parent to find us, let alone mess with our tree.
* That said, taking our i_mutex is closer to mkdir
* emulation, and shouldn't hurt. */
* emulation, and shouldn't hurt.
*/
mutex_lock(&dentry->d_inode->i_mutex);
for (i = 0; group->default_groups[i]; i++) {
......@@ -546,20 +548,34 @@ static void unlink_obj(struct config_item *item)
item->ci_group = NULL;
item->ci_parent = NULL;
/* Drop the reference for ci_entry */
config_item_put(item);
/* Drop the reference for ci_parent */
config_group_put(group);
}
}
static void link_obj(struct config_item *parent_item, struct config_item *item)
{
/* Parent seems redundant with group, but it makes certain
* traversals much nicer. */
/*
* Parent seems redundant with group, but it makes certain
* traversals much nicer.
*/
item->ci_parent = parent_item;
/*
* We hold a reference on the parent for the child's ci_parent
* link.
*/
item->ci_group = config_group_get(to_config_group(parent_item));
list_add_tail(&item->ci_entry, &item->ci_group->cg_children);
/*
* We hold a reference on the child for ci_entry on the parent's
* cg_children
*/
config_item_get(item);
}
......@@ -684,6 +700,10 @@ static void client_drop_item(struct config_item *parent_item,
type = parent_item->ci_type;
BUG_ON(!type);
/*
* If ->drop_item() exists, it is responsible for the
* config_item_put().
*/
if (type->ct_group_ops && type->ct_group_ops->drop_item)
type->ct_group_ops->drop_item(to_config_group(parent_item),
item);
......@@ -694,14 +714,14 @@ static void client_drop_item(struct config_item *parent_item,
static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
{
int ret;
int ret, module_got = 0;
struct config_group *group;
struct config_item *item;
struct config_item *parent_item;
struct configfs_subsystem *subsys;
struct configfs_dirent *sd;
struct config_item_type *type;
struct module *owner;
struct module *owner = NULL;
char *name;
if (dentry->d_parent == configfs_sb->s_root) {
......@@ -754,43 +774,63 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
kfree(name);
if (!item) {
/*
* If item == NULL, then link_obj() was never called.
* There are no extra references to clean up.
*/
ret = -ENOMEM;
goto out_put;
}
ret = -EINVAL;
/*
* link_obj() has been called (via link_group() for groups).
* From here on out, errors must clean that up.
*/
type = item->ci_type;
if (type) {
owner = type->ct_owner;
if (try_module_get(owner)) {
if (group) {
ret = configfs_attach_group(parent_item,
item,
dentry);
} else {
ret = configfs_attach_item(parent_item,
item,
dentry);
}
if (!type) {
ret = -EINVAL;
goto out_unlink;
}
if (ret) {
down(&subsys->su_sem);
if (group)
unlink_group(group);
else
unlink_obj(item);
client_drop_item(parent_item, item);
up(&subsys->su_sem);
owner = type->ct_owner;
if (!try_module_get(owner)) {
ret = -EINVAL;
goto out_unlink;
}
module_put(owner);
}
}
/*
* I hate doing it this way, but if there is
* an error, module_put() probably should
* happen after any cleanup.
*/
module_got = 1;
if (group)
ret = configfs_attach_group(parent_item, item, dentry);
else
ret = configfs_attach_item(parent_item, item, dentry);
out_unlink:
if (ret) {
/* Tear down everything we built up */
down(&subsys->su_sem);
if (group)
unlink_group(group);
else
unlink_obj(item);
client_drop_item(parent_item, item);
up(&subsys->su_sem);
if (module_got)
module_put(owner);
}
out_put:
/*
* link_obj()/link_group() took a reference from child->parent.
* Drop our working ref
* link_obj()/link_group() took a reference from child->parent,
* so the parent is safely pinned. We can drop our working
* reference.
*/
config_item_put(parent_item);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment