Commit 9a73d78cda750f12e25eb811878f2d9dbab1bc6e
Committed by
Mark Fasheh
1 parent
4768e9b18d
Exists in
master
and in
4 other branches
[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 <louis.rilling@kerlabs.com> Signed-off-by: Joel Becker <joel.becker@oracle.com> Signed-off-by: Mark Fasheh <mfasheh@suse.com>
Showing 3 changed files with 16 additions and 0 deletions Side-by-side Diff
fs/configfs/configfs_internal.h
fs/configfs/dir.c
... | ... | @@ -1207,6 +1207,11 @@ |
1207 | 1207 | return -EINVAL; |
1208 | 1208 | } |
1209 | 1209 | |
1210 | + /* | |
1211 | + * Ensure that no racing symlink() will make detach_prep() fail while | |
1212 | + * the new link is temporarily attached | |
1213 | + */ | |
1214 | + mutex_lock(&configfs_symlink_mutex); | |
1210 | 1215 | spin_lock(&configfs_dirent_lock); |
1211 | 1216 | do { |
1212 | 1217 | struct mutex *wait_mutex; |
... | ... | @@ -1215,6 +1220,7 @@ |
1215 | 1220 | if (ret) { |
1216 | 1221 | configfs_detach_rollback(dentry); |
1217 | 1222 | spin_unlock(&configfs_dirent_lock); |
1223 | + mutex_unlock(&configfs_symlink_mutex); | |
1218 | 1224 | if (ret != -EAGAIN) { |
1219 | 1225 | config_item_put(parent_item); |
1220 | 1226 | return ret; |
1221 | 1227 | |
... | ... | @@ -1224,10 +1230,12 @@ |
1224 | 1230 | mutex_lock(wait_mutex); |
1225 | 1231 | mutex_unlock(wait_mutex); |
1226 | 1232 | |
1233 | + mutex_lock(&configfs_symlink_mutex); | |
1227 | 1234 | spin_lock(&configfs_dirent_lock); |
1228 | 1235 | } |
1229 | 1236 | } while (ret == -EAGAIN); |
1230 | 1237 | spin_unlock(&configfs_dirent_lock); |
1238 | + mutex_unlock(&configfs_symlink_mutex); | |
1231 | 1239 | |
1232 | 1240 | /* Get a working ref for the duration of this function */ |
1233 | 1241 | item = configfs_get_config_item(dentry); |
1234 | 1242 | |
... | ... | @@ -1517,11 +1525,13 @@ |
1517 | 1525 | mutex_lock_nested(&configfs_sb->s_root->d_inode->i_mutex, |
1518 | 1526 | I_MUTEX_PARENT); |
1519 | 1527 | mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD); |
1528 | + mutex_lock(&configfs_symlink_mutex); | |
1520 | 1529 | spin_lock(&configfs_dirent_lock); |
1521 | 1530 | if (configfs_detach_prep(dentry, NULL)) { |
1522 | 1531 | printk(KERN_ERR "configfs: Tried to unregister non-empty subsystem!\n"); |
1523 | 1532 | } |
1524 | 1533 | spin_unlock(&configfs_dirent_lock); |
1534 | + mutex_unlock(&configfs_symlink_mutex); | |
1525 | 1535 | configfs_detach_group(&group->cg_item); |
1526 | 1536 | dentry->d_inode->i_flags |= S_DEAD; |
1527 | 1537 | mutex_unlock(&dentry->d_inode->i_mutex); |
fs/configfs/symlink.c
... | ... | @@ -31,6 +31,9 @@ |
31 | 31 | #include <linux/configfs.h> |
32 | 32 | #include "configfs_internal.h" |
33 | 33 | |
34 | +/* Protects attachments of new symlinks */ | |
35 | +DEFINE_MUTEX(configfs_symlink_mutex); | |
36 | + | |
34 | 37 | static int item_depth(struct config_item * item) |
35 | 38 | { |
36 | 39 | struct config_item * p = item; |
37 | 40 | |
... | ... | @@ -147,7 +150,9 @@ |
147 | 150 | |
148 | 151 | ret = type->ct_item_ops->allow_link(parent_item, target_item); |
149 | 152 | if (!ret) { |
153 | + mutex_lock(&configfs_symlink_mutex); | |
150 | 154 | ret = create_link(parent_item, target_item, dentry); |
155 | + mutex_unlock(&configfs_symlink_mutex); | |
151 | 156 | if (ret && type->ct_item_ops->drop_link) |
152 | 157 | type->ct_item_ops->drop_link(parent_item, |
153 | 158 | target_item); |