[PATCH 1/3] virtiofsd: Set up posix_lock hash table for root inode

Vivek Goyal posted 3 patches 5 years, 2 months ago
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>
[PATCH 1/3] virtiofsd: Set up posix_lock hash table for root inode
Posted by Vivek Goyal 5 years, 2 months ago
We setup per inode hash table ->posix_lock to support remote posix locks.
But we forgot to initialize this table for root inode.

Laszlo managed to trigger an issue where he sent a FUSE_FLUSH request for
root inode and lo_flush() found inode with inode->posix_lock NULL and
accessing this table crashed virtiofsd.

May be we can get rid of initializing this hash table for directory
objects completely. But that optimization is for another day.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 97485b22b4..59202a843b 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3372,6 +3372,9 @@ static void setup_root(struct lo_data *lo, struct lo_inode *root)
     root->key.mnt_id = mnt_id;
     root->nlookup = 2;
     g_atomic_int_set(&root->refcount, 2);
+    pthread_mutex_init(&root->plock_mutex, NULL);
+    root->posix_locks = g_hash_table_new_full(
+        g_direct_hash, g_direct_equal, NULL, posix_locks_value_destroy);
 }
 
 static guint lo_key_hash(gconstpointer key)
@@ -3394,6 +3397,10 @@ static void fuse_lo_data_cleanup(struct lo_data *lo)
     if (lo->inodes) {
         g_hash_table_destroy(lo->inodes);
     }
+
+    if (lo->root.posix_locks)
+       g_hash_table_destroy(lo->root.posix_locks);
+
     lo_map_destroy(&lo->fd_map);
     lo_map_destroy(&lo->dirp_map);
     lo_map_destroy(&lo->ino_map);
-- 
2.25.4


Re: [PATCH 1/3] virtiofsd: Set up posix_lock hash table for root inode
Posted by Vivek Goyal 5 years, 2 months ago
We setup per inode hash table ->posix_lock to support remote posix locks.
But we forgot to initialize this table for root inode.

Laszlo managed to trigger an issue where he sent a FUSE_FLUSH request for
root inode and lo_flush() found inode with inode->posix_lock NULL and
accessing this table crashed virtiofsd.

May be we can get rid of initializing this hash table for directory
objects completely. But that optimization is for another day.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c |    7 +++++++
 1 file changed, 7 insertions(+)

Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
===================================================================
--- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c	2020-12-07 14:46:22.198359486 -0500
+++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c	2020-12-07 14:48:07.873737472 -0500
@@ -3372,6 +3372,9 @@ static void setup_root(struct lo_data *l
     root->key.mnt_id = mnt_id;
     root->nlookup = 2;
     g_atomic_int_set(&root->refcount, 2);
+    pthread_mutex_init(&root->plock_mutex, NULL);
+    root->posix_locks = g_hash_table_new_full(
+        g_direct_hash, g_direct_equal, NULL, posix_locks_value_destroy);
 }
 
 static guint lo_key_hash(gconstpointer key)
@@ -3394,6 +3397,10 @@ static void fuse_lo_data_cleanup(struct
     if (lo->inodes) {
         g_hash_table_destroy(lo->inodes);
     }
+
+    if (lo->root.posix_locks) {
+        g_hash_table_destroy(lo->root.posix_locks);
+    }
     lo_map_destroy(&lo->fd_map);
     lo_map_destroy(&lo->dirp_map);
     lo_map_destroy(&lo->ino_map);


Re: [PATCH 1/3] virtiofsd: Set up posix_lock hash table for root inode
Posted by Dr. David Alan Gilbert 5 years, 2 months ago
* Vivek Goyal (vgoyal@redhat.com) wrote:
> We setup per inode hash table ->posix_lock to support remote posix locks.
> But we forgot to initialize this table for root inode.
> 
> Laszlo managed to trigger an issue where he sent a FUSE_FLUSH request for
> root inode and lo_flush() found inode with inode->posix_lock NULL and
> accessing this table crashed virtiofsd.
> 
> May be we can get rid of initializing this hash table for directory
> objects completely. But that optimization is for another day.
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  tools/virtiofsd/passthrough_ll.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
> ===================================================================
> --- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c	2020-12-07 14:46:22.198359486 -0500
> +++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c	2020-12-07 14:48:07.873737472 -0500
> @@ -3372,6 +3372,9 @@ static void setup_root(struct lo_data *l
>      root->key.mnt_id = mnt_id;
>      root->nlookup = 2;
>      g_atomic_int_set(&root->refcount, 2);
> +    pthread_mutex_init(&root->plock_mutex, NULL);
> +    root->posix_locks = g_hash_table_new_full(
> +        g_direct_hash, g_direct_equal, NULL, posix_locks_value_destroy);
>  }
>  
>  static guint lo_key_hash(gconstpointer key)
> @@ -3394,6 +3397,10 @@ static void fuse_lo_data_cleanup(struct
>      if (lo->inodes) {
>          g_hash_table_destroy(lo->inodes);
>      }
> +
> +    if (lo->root.posix_locks) {
> +        g_hash_table_destroy(lo->root.posix_locks);
> +    }
>      lo_map_destroy(&lo->fd_map);
>      lo_map_destroy(&lo->dirp_map);
>      lo_map_destroy(&lo->ino_map);
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK