configfs: fix lockless traversals of ->s_children

Having the parent directory locked protects entries from removal
by another thread, but it does *not* protect cursors from being
moved around by lseek() - or freed, for that matter.

Fixes: 6f6107640625 ("configfs: Introduce configfs_dirent_lock")
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index e84483a0836d8e88bf94adde476414b05b1e1893..eb991b2a9c341f8b14a750e5dedd41b5ad84f9e7 100644 (file)
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -235,15 +235,16 @@ static int configfs_dirent_exists(struct dentry *dentry)
        const unsigned char *new = dentry->d_name.name;
        struct configfs_dirent *sd;
 
+       spin_lock(&configfs_dirent_lock);
        list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
                if (sd->s_element) {
-                       const unsigned char *existing = configfs_get_name(sd);
-                       if (strcmp(existing, new))
-                               continue;
-                       else
+                       if (strcmp(configfs_get_name(sd), new) == 0) {
+                               spin_unlock(&configfs_dirent_lock);
                                return -EEXIST;
+                       }
                }
        }
+       spin_unlock(&configfs_dirent_lock);
 
        return 0;
 }
@@ -575,11 +576,28 @@ static void configfs_detach_rollback(struct dentry *dentry)
                        configfs_detach_rollback(sd->s_dentry);
 }
 
+/*
+ * Find the next non-cursor.  configfs_dirent_lock held by caller.
+ */
+static struct configfs_dirent *next_dirent(struct configfs_dirent *parent,
+                                          struct configfs_dirent *last)
+{
+       struct configfs_dirent *s;
+
+       s = list_prepare_entry(last, &parent->s_children, s_sibling);
+
+       list_for_each_entry_continue(s, &parent->s_children, s_sibling) {
+               if (s->s_element)
+                       return s;
+       }
+       return NULL;
+}
+
 static void detach_attrs(struct config_item * item)
 {
        struct dentry * dentry = dget(item->ci_dentry);
-       struct configfs_dirent * parent_sd;
-       struct configfs_dirent * sd, * tmp;
+       struct configfs_dirent *parent_sd;
+       struct configfs_dirent *sd, *next;
 
        if (!dentry)
                return;
@@ -588,15 +606,19 @@ static void detach_attrs(struct config_item * item)
                 dentry->d_name.name);
 
        parent_sd = dentry->d_fsdata;
-       list_for_each_entry_safe(sd, tmp, &parent_sd->s_children, s_sibling) {
-               if (!sd->s_element || !(sd->s_type & CONFIGFS_NOT_PINNED))
+
+       spin_lock(&configfs_dirent_lock);
+       for (sd = next_dirent(parent_sd, NULL); sd; sd = next) {
+               next = next_dirent(parent_sd, sd);
+               if (!(sd->s_type & CONFIGFS_NOT_PINNED))
                        continue;
-               spin_lock(&configfs_dirent_lock);
                list_del_init(&sd->s_sibling);
                spin_unlock(&configfs_dirent_lock);
                configfs_drop_dentry(sd, dentry);
                configfs_put(sd);
+               spin_lock(&configfs_dirent_lock);
        }
+       spin_unlock(&configfs_dirent_lock);
 
        /**
         * Drop reference from dget() on entrance.
@@ -655,18 +677,20 @@ static void detach_groups(struct config_group *group)
        struct dentry * dentry = dget(group->cg_item.ci_dentry);
        struct dentry *child;
        struct configfs_dirent *parent_sd;
-       struct configfs_dirent *sd, *tmp;
+       struct configfs_dirent *sd, *next;
 
        if (!dentry)
                return;
 
        parent_sd = dentry->d_fsdata;
-       list_for_each_entry_safe(sd, tmp, &parent_sd->s_children, s_sibling) {
-               if (!sd->s_element ||
-                   !(sd->s_type & CONFIGFS_USET_DEFAULT))
+       spin_lock(&configfs_dirent_lock);
+       for (sd = next_dirent(parent_sd, NULL); sd; sd = next) {
+               next = next_dirent(parent_sd, sd);
+               if (!(sd->s_type & CONFIGFS_USET_DEFAULT))
                        continue;
 
                child = sd->s_dentry;
+               spin_unlock(&configfs_dirent_lock);
 
                inode_lock(d_inode(child));
 
@@ -678,7 +702,9 @@ static void detach_groups(struct config_group *group)
 
                d_delete(child);
                dput(child);
+               spin_lock(&configfs_dirent_lock);
        }
+       spin_unlock(&configfs_dirent_lock);
 
        /**
         * Drop reference from dget() on entrance.
@@ -1130,6 +1156,7 @@ configfs_find_subsys_dentry(struct configfs_dirent *root_sd,
        struct configfs_dirent *p;
        struct configfs_dirent *ret = NULL;
 
+       spin_lock(&configfs_dirent_lock);
        list_for_each_entry(p, &root_sd->s_children, s_sibling) {
                if (p->s_type & CONFIGFS_DIR &&
                    p->s_element == subsys_item) {
@@ -1137,6 +1164,7 @@ configfs_find_subsys_dentry(struct configfs_dirent *root_sd,
                        break;
                }
        }
+       spin_unlock(&configfs_dirent_lock);
 
        return ret;
 }