From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: virtio-fs@redhat.com, lersek@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Virtio-fs] [PATCH 1/3] virtiofsd: Set up posix_lock hash table for root inode
Date: Thu, 10 Dec 2020 19:50:35 +0000 [thread overview]
Message-ID: <20201210195035.GD12670@work-vm> (raw)
In-Reply-To: <20201207195539.GB3107@redhat.com>
* 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
WARNING: multiple messages have this Message-ID (diff)
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: virtio-fs@redhat.com, mszeredi@redhat.com, lersek@redhat.com,
qemu-devel@nongnu.org, stefanha@redhat.com
Subject: Re: [PATCH 1/3] virtiofsd: Set up posix_lock hash table for root inode
Date: Thu, 10 Dec 2020 19:50:35 +0000 [thread overview]
Message-ID: <20201210195035.GD12670@work-vm> (raw)
In-Reply-To: <20201207195539.GB3107@redhat.com>
* 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
next prev parent reply other threads:[~2020-12-10 19:50 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-07 18:30 [Virtio-fs] [PATCH 0/3] virtiofsd: Fix lo_flush() and inode->posix_lock init Vivek Goyal
2020-12-07 18:30 ` Vivek Goyal
2020-12-07 18:30 ` [Virtio-fs] [PATCH 1/3] virtiofsd: Set up posix_lock hash table for root inode Vivek Goyal
2020-12-07 18:30 ` Vivek Goyal
2020-12-07 19:55 ` [Virtio-fs] " Vivek Goyal
2020-12-07 19:55 ` Vivek Goyal
2020-12-10 19:50 ` Dr. David Alan Gilbert [this message]
2020-12-10 19:50 ` Dr. David Alan Gilbert
2020-12-07 18:30 ` [Virtio-fs] [PATCH 2/3] virtiofsd: Disable posix_lock hash table if remote locks are not enabled Vivek Goyal
2020-12-07 18:30 ` Vivek Goyal
2020-12-10 19:58 ` [Virtio-fs] " Dr. David Alan Gilbert
2020-12-10 19:58 ` Dr. David Alan Gilbert
2020-12-07 18:30 ` [Virtio-fs] [PATCH 3/3] virtiofsd: Check file type in lo_flush() Vivek Goyal
2020-12-07 18:30 ` Vivek Goyal
2020-12-10 20:03 ` [Virtio-fs] " Dr. David Alan Gilbert
2020-12-10 20:03 ` Dr. David Alan Gilbert
2020-12-10 20:09 ` [Virtio-fs] " Vivek Goyal
2020-12-10 20:09 ` Vivek Goyal
2020-12-10 20:14 ` [Virtio-fs] " Dr. David Alan Gilbert
2020-12-10 20:14 ` Dr. David Alan Gilbert
2020-12-11 14:25 ` [Virtio-fs] " Vivek Goyal
2020-12-11 14:25 ` Vivek Goyal
2020-12-11 19:54 ` [Virtio-fs] " Dr. David Alan Gilbert
2020-12-11 19:54 ` Dr. David Alan Gilbert
2020-12-10 21:24 ` [Virtio-fs] ceph + freeipa ubuntu/fedora common small bug Harry G. Coin
2020-12-10 21:24 ` Harry G. Coin
2020-12-11 11:05 ` [Virtio-fs] " Dr. David Alan Gilbert
2020-12-11 11:05 ` Dr. David Alan Gilbert
2020-12-11 15:06 ` [Virtio-fs] " Vivek Goyal
2020-12-11 15:06 ` Vivek Goyal
2020-12-12 6:39 ` [Virtio-fs] " Harry Coin
2020-12-12 6:39 ` Harry Coin
2020-12-07 19:12 ` [Virtio-fs] [PATCH 0/3] virtiofsd: Fix lo_flush() and inode->posix_lock init no-reply
2020-12-07 19:12 ` no-reply
2020-12-08 4:51 ` [Virtio-fs] " Laszlo Ersek
2020-12-08 4:51 ` Laszlo Ersek
2020-12-08 14:16 ` [Virtio-fs] " Vivek Goyal
2020-12-08 14:16 ` Vivek Goyal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201210195035.GD12670@work-vm \
--to=dgilbert@redhat.com \
--cc=lersek@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=vgoyal@redhat.com \
--cc=virtio-fs@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.