[Qemu-devel] [PATCH 1/2] 9p-synth: fix read-side critical sections

[Paolo Bonzini]

The read-side critical sections in 9p-synth currently only include the
navigation of the list.  This is incorrect; it works for two reasons,
first obviously because rcu_read_lock/unlock are still no-ops; second,
because elements of the list are never deleted from the list (only added).
In fact, only adding items is the reason why rcu_read_lock/unlock can
be left as no-ops.

If items were deleted, they could be reclaimed as soon as the read-side
critical section ends.  So, the read-side critical section must include
all _usage_ of the node we got from the list too.

Signed-off-by: Paolo Bonzini <address@hidden>
---
 hw/9pfs/virtio-9p-synth.c | 35 ++++++++++++++++++++---------------
 1 file modificato, 20 inserzioni(+), 15 rimozioni(-)

diff --git a/hw/9pfs/virtio-9p-synth.c b/hw/9pfs/virtio-9p-synth.c
index 92e0b09..a91ebe1 100644
--- a/hw/9pfs/virtio-9p-synth.c
+++ b/hw/9pfs/virtio-9p-synth.c
@@ -237,14 +237,15 @@ static int v9fs_synth_get_dentry(V9fsSynthNode *dir, 
struct dirent *entry,
         }
         i++;
     }
-    rcu_read_unlock();
     if (!node) {
         /* end of directory */
         *result = NULL;
-        return 0;
+        goto out;
     }
     v9fs_synth_direntry(node, entry, off);
     *result = entry;
+out:
+    rcu_read_unlock();
     return 0;
 }
 
@@ -466,6 +467,7 @@ static int v9fs_synth_name_to_path(FsContext *ctx, V9fsPath 
*dir_path,
 {
     V9fsSynthNode *node;
     V9fsSynthNode *dir_node;
+    int ret = 0;
 
     /* "." and ".." are not allowed */
     if (!strcmp(name, ".") || !strcmp(name, "..")) {
@@ -473,34 +475,37 @@ static int v9fs_synth_name_to_path(FsContext *ctx, 
V9fsPath *dir_path,
         return -1;
 
     }
+
+    rcu_read_lock();
     if (!dir_path) {
         dir_node = &v9fs_synth_root;
     } else {
         dir_node = *(V9fsSynthNode **)dir_path->data;
     }
-    if (!strcmp(name, "/")) {
-        node = dir_node;
-        goto out;
-    }
-    /* search for the name in the childern */
-    rcu_read_lock();
-    QLIST_FOREACH(node, &dir_node->child, sibling) {
-        if (!strcmp(node->name, name)) {
-            break;
+
+    node = dir_node;
+    if (strcmp(name, "/") != 0) {
+        /* search for the name in the childern */
+        QLIST_FOREACH(node, &dir_node->child, sibling) {
+            if (!strcmp(node->name, name)) {
+                break;
+            }
         }
     }
-    rcu_read_unlock();
 
     if (!node) {
         errno = ENOENT;
-        return -1;
+        ret = -1;
+        goto err_out;
     }
-out:
+
     /* Copy the node pointer to fid */
     target->data = g_malloc(sizeof(void *));
     memcpy(target->data, &node, sizeof(void *));
     target->size = sizeof(void *);
-    return 0;
+err_out:
+    rcu_read_unlock();
+    return ret;
 }
 
 static int v9fs_synth_renameat(FsContext *ctx, V9fsPath *olddir,
-- 
1.7.11.2