[RFC PATCH 1/1] fuse: disable local irqs when processing vq completions
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2021-08-24 11:00:29
Also in:
lkml
Subsystem:
filesystems (vfs and infrastructure), fuse: filesystem in userspace, the rest, virtio file system · Maintainers:
Alexander Viro, Christian Brauner, Miklos Szeredi, Linus Torvalds, German Maglione, Vivek Goyal, Stefan Hajnoczi
The virtqueue completion handler function runs on a work queue and local irqs are still enabled. There is a race where the completion handler function grabs the next completed request just before vring_interrupt() runs. vring_interrupt() sees an empty virtqueue and returns IRQ_NONE, falsely declaring this interrupt unhandled. The unhandled irq causes the kernel to disable the irq: irq 77: nobody cared (try booting with the "irqpoll" option) ... handlers: [<00000000d33eeed7>] vring_interrupt Disabling IRQ #77 The driver hangs afterwards since virtqueue irqs are now ignored. Disable local irqs before calling virtqueue_get_buf() and re-enable them afterwards so that vring_interrupt() doesn't run during the race window. Reported-by: Xiaoling Gao <redacted> Cc: Michael Tsirkin <mst@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- I'm not 100% convinced this fixes everything because vring_interrupt() can still run after our critical section and find the virtqueue empty. virtqueue_disable_cb() should minimize that but it's only a hint and there is a small window when the race condition can happen before it's called. --- fs/fuse/virtio_fs.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 8f52cdaa8445..57e1f264b0a8 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c@@ -319,9 +319,10 @@ static void virtio_fs_hiprio_done_work(struct work_struct *work) struct virtio_fs_vq *fsvq = container_of(work, struct virtio_fs_vq, done_work); struct virtqueue *vq = fsvq->vq; + unsigned long flags; /* Free completed FUSE_FORGET requests */ - spin_lock(&fsvq->lock); + spin_lock_irqsave(&fsvq->lock, flags); do { unsigned int len; void *req;
@@ -333,7 +334,7 @@ static void virtio_fs_hiprio_done_work(struct work_struct *work) dec_in_flight_req(fsvq); } } while (!virtqueue_enable_cb(vq) && likely(!virtqueue_is_broken(vq))); - spin_unlock(&fsvq->lock); + spin_unlock_irqrestore(&fsvq->lock, flags); } static void virtio_fs_request_dispatch_work(struct work_struct *work)
@@ -601,11 +602,15 @@ static void virtio_fs_requests_done_work(struct work_struct *work) struct virtqueue *vq = fsvq->vq; struct fuse_req *req; struct fuse_req *next; + unsigned long flags; unsigned int len; LIST_HEAD(reqs); - /* Collect completed requests off the virtqueue */ - spin_lock(&fsvq->lock); + /* + * Collect completed requests off the virtqueue with irqs disabled to + * prevent races with vring_interrupt(). + */ + spin_lock_irqsave(&fsvq->lock, flags); do { virtqueue_disable_cb(vq);
@@ -615,7 +620,7 @@ static void virtio_fs_requests_done_work(struct work_struct *work) spin_unlock(&fpq->lock); } } while (!virtqueue_enable_cb(vq) && likely(!virtqueue_is_broken(vq))); - spin_unlock(&fsvq->lock); + spin_unlock_irqrestore(&fsvq->lock, flags); /* End requests */ list_for_each_entry_safe(req, next, &reqs, list) {
--
2.31.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization