Skip to content

Commit

Permalink
configfs: Fix failing mkdir() making racing rmdir() fail
Browse files Browse the repository at this point in the history
When fixing the rename() vs rmdir() deadlock, we stopped locking default groups'
inodes in configfs_detach_prep(), letting racing mkdir() in default groups
proceed concurrently. This enables races like below happen, which leads to a
failing mkdir() making rmdir() fail, despite the group to remove having no
user-created directory under it in the end.

	process A: 			process B:
	/* PWD=A/B */
	mkdir("C")
	  make_item("C")
	  attach_group("C")
					rmdir("A")
					  detach_prep("A")
					    detach_prep("B")
					      error because of "C"
					  return -ENOTEMPTY
	    attach_group("C/D")
	      error (eg -ENOMEM)
	  return -ENOMEM

This patch prevents such scenarii by making rmdir() wait as long as
detach_prep() fails because a racing mkdir() is in the middle of attach_group().
To achieve this, mkdir() sets a flag CONFIGFS_USET_IN_MKDIR in parent's
configfs_dirent before calling attach_group(), and clears the flag once
attach_group() is done. detach_prep() fails with -EAGAIN whenever the flag is
hit and returns the guilty inode's mutex so that rmdir() can wait on it.

Signed-off-by: Louis Rilling <Louis.Rilling@kerlabs.com>
Signed-off-by: Joel Becker <joel.becker@oracle.com>
  • Loading branch information
Louis Rilling authored and Mark Fasheh committed Jul 14, 2008
1 parent b3e76af commit 6d8344b
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 10 deletions.
1 change: 1 addition & 0 deletions fs/configfs/configfs_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ struct configfs_dirent {
#define CONFIGFS_USET_DIR 0x0040
#define CONFIGFS_USET_DEFAULT 0x0080
#define CONFIGFS_USET_DROPPING 0x0100
#define CONFIGFS_USET_IN_MKDIR 0x0200
#define CONFIGFS_NOT_PINNED (CONFIGFS_ITEM_ATTR)

extern spinlock_t configfs_dirent_lock;
Expand Down
53 changes: 43 additions & 10 deletions fs/configfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ static struct dentry * configfs_lookup(struct inode *dir,
* If there is an error, the caller will reset the flags via
* configfs_detach_rollback().
*/
static int configfs_detach_prep(struct dentry *dentry)
static int configfs_detach_prep(struct dentry *dentry, struct mutex **wait_mutex)
{
struct configfs_dirent *parent_sd = dentry->d_fsdata;
struct configfs_dirent *sd;
Expand All @@ -379,14 +379,20 @@ static int configfs_detach_prep(struct dentry *dentry)
if (sd->s_type & CONFIGFS_NOT_PINNED)
continue;
if (sd->s_type & CONFIGFS_USET_DEFAULT) {
/* Abort if racing with mkdir() */
if (sd->s_type & CONFIGFS_USET_IN_MKDIR) {
if (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
* deep nesting of default_groups
*/
ret = configfs_detach_prep(sd->s_dentry);
ret = configfs_detach_prep(sd->s_dentry, wait_mutex);
if (!ret)
continue;
} else
Expand Down Expand Up @@ -1113,11 +1119,26 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
*/
module_got = 1;

/*
* Make racing rmdir() fail if it did not tag parent with
* CONFIGFS_USET_DROPPING
* Note: if CONFIGFS_USET_DROPPING is already set, attach_group() will
* fail and let rmdir() terminate correctly
*/
spin_lock(&configfs_dirent_lock);
/* This will make configfs_detach_prep() fail */
sd->s_type |= CONFIGFS_USET_IN_MKDIR;
spin_unlock(&configfs_dirent_lock);

if (group)
ret = configfs_attach_group(parent_item, item, dentry);
else
ret = configfs_attach_item(parent_item, item, dentry);

spin_lock(&configfs_dirent_lock);
sd->s_type &= ~CONFIGFS_USET_IN_MKDIR;
spin_unlock(&configfs_dirent_lock);

out_unlink:
if (ret) {
/* Tear down everything we built up */
Expand Down Expand Up @@ -1182,13 +1203,25 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry)
}

spin_lock(&configfs_dirent_lock);
ret = configfs_detach_prep(dentry);
if (ret) {
configfs_detach_rollback(dentry);
spin_unlock(&configfs_dirent_lock);
config_item_put(parent_item);
return ret;
}
do {
struct mutex *wait_mutex;

ret = configfs_detach_prep(dentry, &wait_mutex);
if (ret) {
configfs_detach_rollback(dentry);
spin_unlock(&configfs_dirent_lock);
if (ret != -EAGAIN) {
config_item_put(parent_item);
return ret;
}

/* Wait until the racing operation terminates */
mutex_lock(wait_mutex);
mutex_unlock(wait_mutex);

spin_lock(&configfs_dirent_lock);
}
} while (ret == -EAGAIN);
spin_unlock(&configfs_dirent_lock);

/* Get a working ref for the duration of this function */
Expand Down Expand Up @@ -1480,7 +1513,7 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys)
I_MUTEX_PARENT);
mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD);
spin_lock(&configfs_dirent_lock);
if (configfs_detach_prep(dentry)) {
if (configfs_detach_prep(dentry, NULL)) {
printk(KERN_ERR "configfs: Tried to unregister non-empty subsystem!\n");
}
spin_unlock(&configfs_dirent_lock);
Expand Down

0 comments on commit 6d8344b

Please sign in to comment.