From dacdd0e04768da1fd2b24a6ee274c582b40d0c5b Mon Sep 17 00:00:00 2001 From: Joel Becker Date: Thu, 17 Jul 2008 16:54:19 -0700 Subject: [PATCH] configfs: Include linux/err.h in linux/configfs.h We now use PTR_ERR() in the ->make_item() and ->make_group() operations. Folks including configfs.h need err.h. Signed-off-by: Joel Becker Signed-off-by: Mark Fasheh --- fs/configfs/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/configfs/dir.c') diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 179589be063a..2495f23e33f4 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -1094,7 +1094,7 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) kfree(name); if (ret) { /* - * If item == NULL, then link_obj() was never called. + * If ret != 0, then link_obj() was never called. * There are no extra references to clean up. */ goto out_put; -- cgit v1.2.2 From 4768e9b18dc63719209c68920d4ae52dc49b6161 Mon Sep 17 00:00:00 2001 From: Louis Rilling Date: Mon, 23 Jun 2008 14:16:17 +0200 Subject: [PATCH] configfs: Fix symlink() to a removing item The rule for configfs symlinks is that symlinks always point to valid config_items, and prevent the target from being removed. However, configfs_symlink() only checks that it can grab a reference on the target item, without ensuring that it remains alive until the symlink is correctly attached. This patch makes configfs_symlink() fail whenever the target is being removed, using the CONFIGFS_USET_DROPPING flag set by configfs_detach_prep() and protected by configfs_dirent_lock. This patch introduces a similar (weird?) behavior as with mkdir failures making rmdir fail: if symlink() races with rmdir() of the parent directory (or its youngest user-created ancestor if parent is a default group) or rmdir() of the target directory, and then fails in configfs_create(), this can make the racing rmdir() fail despite the concerned directory having no user-created entry (resp. no symlink pointing to it or one of its default groups) in the end. This behavior is fixed in later patches. Signed-off-by: Louis Rilling Signed-off-by: Joel Becker Signed-off-by: Mark Fasheh --- fs/configfs/dir.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'fs/configfs/dir.c') diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 2495f23e33f4..cb5ea44846af 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -370,6 +370,9 @@ static int configfs_detach_prep(struct dentry *dentry, struct mutex **wait_mutex struct configfs_dirent *sd; int ret; + /* Mark that we're trying to drop the group */ + parent_sd->s_type |= CONFIGFS_USET_DROPPING; + ret = -EBUSY; if (!list_empty(&parent_sd->s_links)) goto out; @@ -385,8 +388,6 @@ static int configfs_detach_prep(struct dentry *dentry, struct mutex **wait_mutex *wait_mutex = &sd->s_dentry->d_inode->i_mutex; return -EAGAIN; } - /* Mark that we're trying to drop the group */ - sd->s_type |= CONFIGFS_USET_DROPPING; /* * Yup, recursive. If there's a problem, blame @@ -414,12 +415,11 @@ static void configfs_detach_rollback(struct dentry *dentry) struct configfs_dirent *parent_sd = dentry->d_fsdata; struct configfs_dirent *sd; - list_for_each_entry(sd, &parent_sd->s_children, s_sibling) { - if (sd->s_type & CONFIGFS_USET_DEFAULT) { + parent_sd->s_type &= ~CONFIGFS_USET_DROPPING; + + list_for_each_entry(sd, &parent_sd->s_children, s_sibling) + if (sd->s_type & CONFIGFS_USET_DEFAULT) configfs_detach_rollback(sd->s_dentry); - sd->s_type &= ~CONFIGFS_USET_DROPPING; - } - } } static void detach_attrs(struct config_item * item) -- cgit v1.2.2 From 9a73d78cda750f12e25eb811878f2d9dbab1bc6e Mon Sep 17 00:00:00 2001 From: Louis Rilling Date: Fri, 20 Jun 2008 14:09:22 +0200 Subject: [PATCH] configfs: Fix failing symlink() making rmdir() fail On a similar pattern as mkdir() vs rmdir(), a failing symlink() may make rmdir() fail for the symlink's parent and the symlink's target as well. failing symlink() making target's rmdir() fail: process 1: process 2: symlink("A/S" -> "B") allow_link() create_link() attach to "B" links list rmdir("B") detach_prep("B") error because of new link configfs_create_link("A", "S") error (eg -ENOMEM) failing symlink() making parent's rmdir() fail: process 1: process 2: symlink("A/D/S" -> "B") allow_link() create_link() attach to "B" links list configfs_create_link("A/D", "S") make_dirent("A/D", "S") rmdir("A") detach_prep("A") detach_prep("A/D") error because of "S" create("S") error (eg -ENOMEM) We cannot use the same solution as for mkdir() vs rmdir(), since rmdir() on the target cannot wait on the i_mutex of the new symlink's parent without risking a deadlock (with other symlink() or sys_rename()). Instead we define a global mutex protecting all configfs symlinks attachment, so that rmdir() can avoid the races above. Signed-off-by: Louis Rilling Signed-off-by: Joel Becker Signed-off-by: Mark Fasheh --- fs/configfs/dir.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'fs/configfs/dir.c') diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index cb5ea44846af..4e228c80fe9f 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -1207,6 +1207,11 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry) return -EINVAL; } + /* + * Ensure that no racing symlink() will make detach_prep() fail while + * the new link is temporarily attached + */ + mutex_lock(&configfs_symlink_mutex); spin_lock(&configfs_dirent_lock); do { struct mutex *wait_mutex; @@ -1215,6 +1220,7 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry) if (ret) { configfs_detach_rollback(dentry); spin_unlock(&configfs_dirent_lock); + mutex_unlock(&configfs_symlink_mutex); if (ret != -EAGAIN) { config_item_put(parent_item); return ret; @@ -1224,10 +1230,12 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry) mutex_lock(wait_mutex); mutex_unlock(wait_mutex); + mutex_lock(&configfs_symlink_mutex); spin_lock(&configfs_dirent_lock); } } while (ret == -EAGAIN); spin_unlock(&configfs_dirent_lock); + mutex_unlock(&configfs_symlink_mutex); /* Get a working ref for the duration of this function */ item = configfs_get_config_item(dentry); @@ -1517,11 +1525,13 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys) mutex_lock_nested(&configfs_sb->s_root->d_inode->i_mutex, I_MUTEX_PARENT); mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD); + mutex_lock(&configfs_symlink_mutex); spin_lock(&configfs_dirent_lock); if (configfs_detach_prep(dentry, NULL)) { printk(KERN_ERR "configfs: Tried to unregister non-empty subsystem!\n"); } spin_unlock(&configfs_dirent_lock); + mutex_unlock(&configfs_symlink_mutex); configfs_detach_group(&group->cg_item); dentry->d_inode->i_flags |= S_DEAD; mutex_unlock(&dentry->d_inode->i_mutex); -- cgit v1.2.2 From 2a109f2a4155f168047aa2f5b3a170e279bef89a Mon Sep 17 00:00:00 2001 From: Louis Rilling Date: Fri, 4 Jul 2008 16:56:05 +0200 Subject: [PATCH] configfs: Prevent userspace from creating new entries under attaching directories process 1: process 2: configfs_mkdir("A") attach_group("A") attach_item("A") d_instantiate("A") populate_groups("A") mutex_lock("A") attach_group("A/B") attach_item("A") d_instantiate("A/B") mkdir("A/B/C") do_path_lookup("A/B/C", LOOKUP_PARENT) ok lookup_create("A/B/C") mutex_lock("A/B") ok configfs_mkdir("A/B/C") ok attach_group("A/C") attach_item("A/C") d_instantiate("A/C") populate_groups("A/C") mutex_lock("A/C") attach_group("A/C/D") attach_item("A/C/D") failure mutex_unlock("A/C") detach_groups("A/C") nothing to do mkdir("A/C/E") do_path_lookup("A/C/E", LOOKUP_PARENT) ok lookup_create("A/C/E") mutex_lock("A/C") ok configfs_mkdir("A/C/E") ok detach_item("A/C") d_delete("A/C") mutex_unlock("A") detach_groups("A") mutex_lock("A/B") detach_group("A/B") detach_groups("A/B") nothing since no _default_ group detach_item("A/B") mutex_unlock("A/B") d_delete("A/B") detach_item("A") d_delete("A") Two bugs: 1/ "A/B/C" and "A/C/E" are created, but never removed while their parent are removed in the end. The same could happen with symlink() instead of mkdir(). 2/ "A" and "A/C" inodes are not locked while detach_item() is called on them, which may probably confuse VFS. This commit fixes 1/, tagging new directories with CONFIGFS_USET_CREATING before building the inode and instantiating the dentry, and validating the whole group+default groups hierarchy in a second pass by clearing CONFIGFS_USET_CREATING. mkdir(), symlink(), lookup(), and dir_open() simply return -ENOENT if called in (or linking to) a directory tagged with CONFIGFS_USET_CREATING. This does not prevent userspace from calling stat() successfuly on such directories, but this prevents userspace from adding (children to | symlinking from/to | read/write attributes of | listing the contents of) not validated items. In other words, userspace will not interact with the subsystem on a new item until the new item creation completes correctly. It was first proposed to re-use CONFIGFS_USET_IN_MKDIR instead of a new flag CONFIGFS_USET_CREATING, but this generated conflicts when checking the target of a new symlink: a valid target directory in the middle of attaching a new user-created child item could be wrongly detected as being attached. 2/ is fixed by next commit. Signed-off-by: Louis Rilling Signed-off-by: Joel Becker Signed-off-by: Mark Fasheh --- fs/configfs/dir.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 86 insertions(+), 5 deletions(-) (limited to 'fs/configfs/dir.c') diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 4e228c80fe9f..647499a3479b 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -185,7 +185,7 @@ static int create_dir(struct config_item * k, struct dentry * p, error = configfs_dirent_exists(p->d_fsdata, d->d_name.name); if (!error) error = configfs_make_dirent(p->d_fsdata, d, k, mode, - CONFIGFS_DIR); + CONFIGFS_DIR | CONFIGFS_USET_CREATING); if (!error) { error = configfs_create(d, mode, init_dir); if (!error) { @@ -209,6 +209,9 @@ static int create_dir(struct config_item * k, struct dentry * p, * configfs_create_dir - create a directory for an config_item. * @item: config_itemwe're creating directory for. * @dentry: config_item's dentry. + * + * Note: user-created entries won't be allowed under this new directory + * until it is validated by configfs_dir_set_ready() */ static int configfs_create_dir(struct config_item * item, struct dentry *dentry) @@ -231,6 +234,44 @@ static int configfs_create_dir(struct config_item * item, struct dentry *dentry) return error; } +/* + * Allow userspace to create new entries under a new directory created with + * configfs_create_dir(), and under all of its chidlren directories recursively. + * @sd configfs_dirent of the new directory to validate + * + * Caller must hold configfs_dirent_lock. + */ +static void configfs_dir_set_ready(struct configfs_dirent *sd) +{ + struct configfs_dirent *child_sd; + + sd->s_type &= ~CONFIGFS_USET_CREATING; + list_for_each_entry(child_sd, &sd->s_children, s_sibling) + if (child_sd->s_type & CONFIGFS_USET_CREATING) + configfs_dir_set_ready(child_sd); +} + +/* + * Check that a directory does not belong to a directory hierarchy being + * attached and not validated yet. + * @sd configfs_dirent of the directory to check + * + * @return non-zero iff the directory was validated + * + * Note: takes configfs_dirent_lock, so the result may change from false to true + * in two consecutive calls, but never from true to false. + */ +int configfs_dirent_is_ready(struct configfs_dirent *sd) +{ + int ret; + + spin_lock(&configfs_dirent_lock); + ret = !(sd->s_type & CONFIGFS_USET_CREATING); + spin_unlock(&configfs_dirent_lock); + + return ret; +} + int configfs_create_link(struct configfs_symlink *sl, struct dentry *parent, struct dentry *dentry) @@ -330,7 +371,19 @@ static struct dentry * configfs_lookup(struct inode *dir, struct configfs_dirent * parent_sd = dentry->d_parent->d_fsdata; struct configfs_dirent * sd; int found = 0; - int err = 0; + int err; + + /* + * Fake invisibility if dir belongs to a group/default groups hierarchy + * being attached + * + * This forbids userspace to read/write attributes of items which may + * 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; list_for_each_entry(sd, &parent_sd->s_children, s_sibling) { if (sd->s_type & CONFIGFS_NOT_PINNED) { @@ -353,6 +406,7 @@ static struct dentry * configfs_lookup(struct inode *dir, return simple_lookup(dir, dentry, nd); } +out: return ERR_PTR(err); } @@ -1044,6 +1098,16 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) } sd = dentry->d_parent->d_fsdata; + + /* + * Fake invisibility if dir belongs to a group/default groups hierarchy + * being attached + */ + if (!configfs_dirent_is_ready(sd)) { + ret = -ENOENT; + goto out; + } + if (!(sd->s_type & CONFIGFS_USET_DIR)) { ret = -EPERM; goto out; @@ -1142,6 +1206,8 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) spin_lock(&configfs_dirent_lock); sd->s_type &= ~CONFIGFS_USET_IN_MKDIR; + if (!ret) + configfs_dir_set_ready(dentry->d_fsdata); spin_unlock(&configfs_dirent_lock); out_unlink: @@ -1322,13 +1388,24 @@ static int configfs_dir_open(struct inode *inode, struct file *file) { struct dentry * dentry = file->f_path.dentry; struct configfs_dirent * parent_sd = dentry->d_fsdata; + int err; mutex_lock(&dentry->d_inode->i_mutex); - file->private_data = configfs_new_dirent(parent_sd, NULL); + /* + * Fake invisibility if dir belongs to a group/default groups hierarchy + * being attached + */ + err = -ENOENT; + if (configfs_dirent_is_ready(parent_sd)) { + file->private_data = configfs_new_dirent(parent_sd, NULL); + if (IS_ERR(file->private_data)) + err = PTR_ERR(file->private_data); + else + err = 0; + } mutex_unlock(&dentry->d_inode->i_mutex); - return IS_ERR(file->private_data) ? PTR_ERR(file->private_data) : 0; - + return err; } static int configfs_dir_close(struct inode *inode, struct file *file) @@ -1499,6 +1576,10 @@ int configfs_register_subsystem(struct configfs_subsystem *subsys) if (err) { d_delete(dentry); dput(dentry); + } else { + spin_lock(&configfs_dirent_lock); + configfs_dir_set_ready(dentry->d_fsdata); + spin_unlock(&configfs_dirent_lock); } } -- cgit v1.2.2 From 2e2ce171c3ba6f2753fb1fd2706b63683394da2d Mon Sep 17 00:00:00 2001 From: Louis Rilling Date: Fri, 4 Jul 2008 16:56:06 +0200 Subject: [PATCH] configfs: Lock new directory inodes before removing on cleanup after failure Once a new configfs directory is created by configfs_attach_item() or configfs_attach_group(), a failure in the remaining initialization steps leads to removing a directory which inode the VFS may have already accessed. This commit adds the necessary inode locking to safely remove configfs directories while cleaning up after a failure. As an advantage, the locking rules of populate_groups() and detach_groups() become the same: the caller must have the group's inode mutex locked. Signed-off-by: Louis Rilling Signed-off-by: Joel Becker Signed-off-by: Mark Fasheh --- fs/configfs/dir.c | 48 +++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 19 deletions(-) (limited to 'fs/configfs/dir.c') diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 647499a3479b..4d11479cf2c3 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -324,6 +324,8 @@ static void remove_dir(struct dentry * d) * The only thing special about this is that we remove any files in * the directory before we remove the directory, and we've inlined * what used to be configfs_rmdir() below, instead of calling separately. + * + * Caller holds the mutex of the item's inode */ static void configfs_remove_dir(struct config_item * item) @@ -612,36 +614,21 @@ static int create_default_group(struct config_group *parent_group, static int populate_groups(struct config_group *group) { struct config_group *new_group; - struct dentry *dentry = group->cg_item.ci_dentry; int ret = 0; int i; if (group->default_groups) { - /* - * 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. - */ - mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD); - for (i = 0; group->default_groups[i]; i++) { new_group = group->default_groups[i]; ret = create_default_group(group, new_group); - if (ret) + if (ret) { + detach_groups(group); break; + } } - - mutex_unlock(&dentry->d_inode->i_mutex); } - if (ret) - detach_groups(group); - return ret; } @@ -756,7 +743,15 @@ static int configfs_attach_item(struct config_item *parent_item, if (!ret) { ret = populate_attrs(item); if (ret) { + /* + * We are going to remove an inode and its dentry but + * the VFS may already have hit and used them. Thus, + * we must lock them as rmdir() would. + */ + mutex_lock(&dentry->d_inode->i_mutex); configfs_remove_dir(item); + dentry->d_inode->i_flags |= S_DEAD; + mutex_unlock(&dentry->d_inode->i_mutex); d_delete(dentry); } } @@ -764,6 +759,7 @@ static int configfs_attach_item(struct config_item *parent_item, return ret; } +/* Caller holds the mutex of the item's inode */ static void configfs_detach_item(struct config_item *item) { detach_attrs(item); @@ -782,16 +778,30 @@ static int configfs_attach_group(struct config_item *parent_item, sd = dentry->d_fsdata; sd->s_type |= CONFIGFS_USET_DIR; + /* + * FYI, we're faking mkdir in populate_groups() + * We must lock the group's inode to avoid races with the VFS + * which can already hit the inode and try to add/remove entries + * under it. + * + * We must also lock the inode to remove it safely in case of + * error, as rmdir() would. + */ + mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD); ret = populate_groups(to_config_group(item)); if (ret) { configfs_detach_item(item); - d_delete(dentry); + dentry->d_inode->i_flags |= S_DEAD; } + mutex_unlock(&dentry->d_inode->i_mutex); + if (ret) + d_delete(dentry); } return ret; } +/* Caller holds the mutex of the group's inode */ static void configfs_detach_group(struct config_item *item) { detach_groups(to_config_group(item)); -- cgit v1.2.2 From 99cefda42ac550863b5ae1df9e60322e377decf9 Mon Sep 17 00:00:00 2001 From: Louis Rilling Date: Fri, 27 Jun 2008 13:10:25 +0200 Subject: [PATCH] configfs: Fix open directory making rmdir() fail When checking for user-created elements under an item to be removed by rmdir(), configfs_detach_prep() counts fake configfs_dirents created by dir_open() as user-created and fails when finding one. It is however perfectly valid to remove a directory that is open. Simply make configfs_detach_prep() skip fake configfs_dirent, like it already does for attributes, and like detach_groups() does. Signed-off-by: Louis Rilling Signed-off-by: Joel Becker Signed-off-by: Mark Fasheh --- fs/configfs/dir.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs/configfs/dir.c') diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 4d11479cf2c3..a89058b39884 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -435,7 +435,8 @@ static int configfs_detach_prep(struct dentry *dentry, struct mutex **wait_mutex ret = 0; list_for_each_entry(sd, &parent_sd->s_children, s_sibling) { - if (sd->s_type & CONFIGFS_NOT_PINNED) + if (!sd->s_element || + (sd->s_type & CONFIGFS_NOT_PINNED)) continue; if (sd->s_type & CONFIGFS_USET_DEFAULT) { /* Abort if racing with mkdir() */ -- cgit v1.2.2 From 70526b67443a980d5029d9cf06903bef731a4e96 Mon Sep 17 00:00:00 2001 From: Joel Becker Date: Tue, 17 Jun 2008 15:34:32 -0700 Subject: [PATCH] configfs: Pin configfs subsystems separately from new config_items. configfs_mkdir() creates a new item by calling its parent's ->make_item/group() functions. Once that object is created, configfs_mkdir() calls try_module_get() on the new item's module. If it succeeds, the module owning the new item cannot be unloaded, and configfs is safe to reference the item. If the item and the subsystem it belongs to are part of the same module, the subsystem is also pinned. This is the common case. However, if the subsystem is made up of multiple modules, this may not pin the subsystem. Thus, it would be possible to unload the toplevel subsystem module while there is still a child item. Thus, we now try_module_get() the subsystem's module. This only really affects children of the toplevel subsystem group. Deeper children already have their parents pinned. Signed-off-by: Joel Becker Signed-off-by: Mark Fasheh --- fs/configfs/dir.c | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) (limited to 'fs/configfs/dir.c') diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index a89058b39884..7a8db78a91d2 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -1100,7 +1100,7 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) struct configfs_subsystem *subsys; struct configfs_dirent *sd; struct config_item_type *type; - struct module *owner = NULL; + struct module *subsys_owner = NULL, *new_item_owner = NULL; char *name; if (dentry->d_parent == configfs_sb->s_root) { @@ -1137,10 +1137,25 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) goto out_put; } + /* + * The subsystem may belong to a different module than the item + * being created. We don't want to safely pin the new item but + * fail to pin the subsystem it sits under. + */ + if (!subsys->su_group.cg_item.ci_type) { + ret = -EINVAL; + goto out_put; + } + subsys_owner = subsys->su_group.cg_item.ci_type->ct_owner; + if (!try_module_get(subsys_owner)) { + ret = -EINVAL; + goto out_put; + } + name = kmalloc(dentry->d_name.len + 1, GFP_KERNEL); if (!name) { ret = -ENOMEM; - goto out_put; + goto out_subsys_put; } snprintf(name, dentry->d_name.len + 1, "%s", dentry->d_name.name); @@ -1172,7 +1187,7 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) * If ret != 0, then link_obj() was never called. * There are no extra references to clean up. */ - goto out_put; + goto out_subsys_put; } /* @@ -1186,8 +1201,8 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) goto out_unlink; } - owner = type->ct_owner; - if (!try_module_get(owner)) { + new_item_owner = type->ct_owner; + if (!try_module_get(new_item_owner)) { ret = -EINVAL; goto out_unlink; } @@ -1236,9 +1251,13 @@ out_unlink: mutex_unlock(&subsys->su_mutex); if (module_got) - module_put(owner); + module_put(new_item_owner); } +out_subsys_put: + if (ret) + module_put(subsys_owner); + out_put: /* * link_obj()/link_group() took a reference from child->parent, @@ -1257,7 +1276,7 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry) struct config_item *item; struct configfs_subsystem *subsys; struct configfs_dirent *sd; - struct module *owner = NULL; + struct module *subsys_owner = NULL, *dead_item_owner = NULL; int ret; if (dentry->d_parent == configfs_sb->s_root) @@ -1284,6 +1303,10 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry) return -EINVAL; } + /* configfs_mkdir() shouldn't have allowed this */ + BUG_ON(!subsys->su_group.cg_item.ci_type); + subsys_owner = subsys->su_group.cg_item.ci_type->ct_owner; + /* * Ensure that no racing symlink() will make detach_prep() fail while * the new link is temporarily attached @@ -1321,7 +1344,7 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry) config_item_put(parent_item); if (item->ci_type) - owner = item->ci_type->ct_owner; + dead_item_owner = item->ci_type->ct_owner; if (sd->s_type & CONFIGFS_USET_DIR) { configfs_detach_group(item); @@ -1343,7 +1366,8 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry) /* Drop our reference from above */ config_item_put(item); - module_put(owner); + module_put(dead_item_owner); + module_put(subsys_owner); return 0; } -- cgit v1.2.2