diff options
author | Louis Rilling <Louis.Rilling@kerlabs.com> | 2008-06-16 13:01:02 -0400 |
---|---|---|
committer | Mark Fasheh <mfasheh@suse.com> | 2008-07-14 16:57:16 -0400 |
commit | 6d8344baee99402de58b5fa5dfea197242955c15 (patch) | |
tree | 6be890feb8063bdaac6efaff1044980cb76ee961 | |
parent | b3e76af87441fc36eef3516d73ab2314e7b2d911 (diff) |
configfs: Fix failing mkdir() making racing rmdir() fail
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>
-rw-r--r-- | fs/configfs/configfs_internal.h | 1 | ||||
-rw-r--r-- | fs/configfs/dir.c | 53 |
2 files changed, 44 insertions, 10 deletions
diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h index 5a33b58e66da..da015c12e3ea 100644 --- a/fs/configfs/configfs_internal.h +++ b/fs/configfs/configfs_internal.h | |||
@@ -48,6 +48,7 @@ struct configfs_dirent { | |||
48 | #define CONFIGFS_USET_DIR 0x0040 | 48 | #define CONFIGFS_USET_DIR 0x0040 |
49 | #define CONFIGFS_USET_DEFAULT 0x0080 | 49 | #define CONFIGFS_USET_DEFAULT 0x0080 |
50 | #define CONFIGFS_USET_DROPPING 0x0100 | 50 | #define CONFIGFS_USET_DROPPING 0x0100 |
51 | #define CONFIGFS_USET_IN_MKDIR 0x0200 | ||
51 | #define CONFIGFS_NOT_PINNED (CONFIGFS_ITEM_ATTR) | 52 | #define CONFIGFS_NOT_PINNED (CONFIGFS_ITEM_ATTR) |
52 | 53 | ||
53 | extern spinlock_t configfs_dirent_lock; | 54 | extern spinlock_t configfs_dirent_lock; |
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index d5b5985716ba..614e382a6049 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c | |||
@@ -364,7 +364,7 @@ static struct dentry * configfs_lookup(struct inode *dir, | |||
364 | * If there is an error, the caller will reset the flags via | 364 | * If there is an error, the caller will reset the flags via |
365 | * configfs_detach_rollback(). | 365 | * configfs_detach_rollback(). |
366 | */ | 366 | */ |
367 | static int configfs_detach_prep(struct dentry *dentry) | 367 | static int configfs_detach_prep(struct dentry *dentry, struct mutex **wait_mutex) |
368 | { | 368 | { |
369 | struct configfs_dirent *parent_sd = dentry->d_fsdata; | 369 | struct configfs_dirent *parent_sd = dentry->d_fsdata; |
370 | struct configfs_dirent *sd; | 370 | struct configfs_dirent *sd; |
@@ -379,6 +379,12 @@ static int configfs_detach_prep(struct dentry *dentry) | |||
379 | if (sd->s_type & CONFIGFS_NOT_PINNED) | 379 | if (sd->s_type & CONFIGFS_NOT_PINNED) |
380 | continue; | 380 | continue; |
381 | if (sd->s_type & CONFIGFS_USET_DEFAULT) { | 381 | if (sd->s_type & CONFIGFS_USET_DEFAULT) { |
382 | /* Abort if racing with mkdir() */ | ||
383 | if (sd->s_type & CONFIGFS_USET_IN_MKDIR) { | ||
384 | if (wait_mutex) | ||
385 | *wait_mutex = &sd->s_dentry->d_inode->i_mutex; | ||
386 | return -EAGAIN; | ||
387 | } | ||
382 | /* Mark that we're trying to drop the group */ | 388 | /* Mark that we're trying to drop the group */ |
383 | sd->s_type |= CONFIGFS_USET_DROPPING; | 389 | sd->s_type |= CONFIGFS_USET_DROPPING; |
384 | 390 | ||
@@ -386,7 +392,7 @@ static int configfs_detach_prep(struct dentry *dentry) | |||
386 | * Yup, recursive. If there's a problem, blame | 392 | * Yup, recursive. If there's a problem, blame |
387 | * deep nesting of default_groups | 393 | * deep nesting of default_groups |
388 | */ | 394 | */ |
389 | ret = configfs_detach_prep(sd->s_dentry); | 395 | ret = configfs_detach_prep(sd->s_dentry, wait_mutex); |
390 | if (!ret) | 396 | if (!ret) |
391 | continue; | 397 | continue; |
392 | } else | 398 | } else |
@@ -1113,11 +1119,26 @@ static int configfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) | |||
1113 | */ | 1119 | */ |
1114 | module_got = 1; | 1120 | module_got = 1; |
1115 | 1121 | ||
1122 | /* | ||
1123 | * Make racing rmdir() fail if it did not tag parent with | ||
1124 | * CONFIGFS_USET_DROPPING | ||
1125 | * Note: if CONFIGFS_USET_DROPPING is already set, attach_group() will | ||
1126 | * fail and let rmdir() terminate correctly | ||
1127 | */ | ||
1128 | spin_lock(&configfs_dirent_lock); | ||
1129 | /* This will make configfs_detach_prep() fail */ | ||
1130 | sd->s_type |= CONFIGFS_USET_IN_MKDIR; | ||
1131 | spin_unlock(&configfs_dirent_lock); | ||
1132 | |||
1116 | if (group) | 1133 | if (group) |
1117 | ret = configfs_attach_group(parent_item, item, dentry); | 1134 | ret = configfs_attach_group(parent_item, item, dentry); |
1118 | else | 1135 | else |
1119 | ret = configfs_attach_item(parent_item, item, dentry); | 1136 | ret = configfs_attach_item(parent_item, item, dentry); |
1120 | 1137 | ||
1138 | spin_lock(&configfs_dirent_lock); | ||
1139 | sd->s_type &= ~CONFIGFS_USET_IN_MKDIR; | ||
1140 | spin_unlock(&configfs_dirent_lock); | ||
1141 | |||
1121 | out_unlink: | 1142 | out_unlink: |
1122 | if (ret) { | 1143 | if (ret) { |
1123 | /* Tear down everything we built up */ | 1144 | /* Tear down everything we built up */ |
@@ -1182,13 +1203,25 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry) | |||
1182 | } | 1203 | } |
1183 | 1204 | ||
1184 | spin_lock(&configfs_dirent_lock); | 1205 | spin_lock(&configfs_dirent_lock); |
1185 | ret = configfs_detach_prep(dentry); | 1206 | do { |
1186 | if (ret) { | 1207 | struct mutex *wait_mutex; |
1187 | configfs_detach_rollback(dentry); | 1208 | |
1188 | spin_unlock(&configfs_dirent_lock); | 1209 | ret = configfs_detach_prep(dentry, &wait_mutex); |
1189 | config_item_put(parent_item); | 1210 | if (ret) { |
1190 | return ret; | 1211 | configfs_detach_rollback(dentry); |
1191 | } | 1212 | spin_unlock(&configfs_dirent_lock); |
1213 | if (ret != -EAGAIN) { | ||
1214 | config_item_put(parent_item); | ||
1215 | return ret; | ||
1216 | } | ||
1217 | |||
1218 | /* Wait until the racing operation terminates */ | ||
1219 | mutex_lock(wait_mutex); | ||
1220 | mutex_unlock(wait_mutex); | ||
1221 | |||
1222 | spin_lock(&configfs_dirent_lock); | ||
1223 | } | ||
1224 | } while (ret == -EAGAIN); | ||
1192 | spin_unlock(&configfs_dirent_lock); | 1225 | spin_unlock(&configfs_dirent_lock); |
1193 | 1226 | ||
1194 | /* Get a working ref for the duration of this function */ | 1227 | /* Get a working ref for the duration of this function */ |
@@ -1480,7 +1513,7 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys) | |||
1480 | I_MUTEX_PARENT); | 1513 | I_MUTEX_PARENT); |
1481 | mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD); | 1514 | mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD); |
1482 | spin_lock(&configfs_dirent_lock); | 1515 | spin_lock(&configfs_dirent_lock); |
1483 | if (configfs_detach_prep(dentry)) { | 1516 | if (configfs_detach_prep(dentry, NULL)) { |
1484 | printk(KERN_ERR "configfs: Tried to unregister non-empty subsystem!\n"); | 1517 | printk(KERN_ERR "configfs: Tried to unregister non-empty subsystem!\n"); |
1485 | } | 1518 | } |
1486 | spin_unlock(&configfs_dirent_lock); | 1519 | spin_unlock(&configfs_dirent_lock); |