* Re: [PATCH] fuse: virtio-fs: don't process completions of aborted requests
2026-10-20 13:53 [PATCH] fuse: virtio-fs: don't process completions of aborted requests Qihang
@ 2026-10-20 14:09 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-10-20 14:09 UTC (permalink / raw)
To: Qihang; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] fuse: virtio-fs: DMA use-after-free on aborted requests in fuse_chan_abort()
- [High] fuse: virtio-fs: fsvq->aborted never cleared in virtio_fs_start_all_queues()
- [High] fuse: virtio-fs: queued_reqs not aborted or drained in virtio_fs_abort()
- [Medium] fuse: virtio-fs: req->argbuf leak on aborted requests in done_work
--
commit 0a788f32822f61f6e1ab378f337037e0c6397f67
Author: Qihang <q.h.hack.winter@gmail.com>
fuse: virtio-fs: don't process completions of aborted requests
This commit adds an abort callback to struct fuse_iqueue_ops so virtio-fs can
mark its virtqueues as aborted when fuse_chan_abort() runs. In
virtio_fs_requests_done_work(), completions on an aborted virtqueue are
skipped after decrementing in_flight to avoid dereferencing requests already
freed by fuse_chan_abort().
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index ba8d79c4bcac6..ae4a14aaaba98 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -55,6 +55,7 @@ struct virtio_fs_vq {
> struct work_struct dispatch_work;
> struct fuse_dev *fud;
> bool connected;
> + bool aborted;
> long in_flight;
> struct completion in_flight_zero; /* No inflight requests */
> struct kobject *kobj;
> @@ -871,6 +872,15 @@ static void virtio_fs_requests_done_work(struct work_struct *work)
> virtqueue_disable_cb(vq);
>
> while ((req = virtqueue_get_buf(vq, &len)) != NULL) {
> + if (fsvq->aborted) {
> + /*
> + * fuse_chan_abort() ended every request this
> + * queue still owns; the token may already be
> + * freed, do not touch it.
> + */
> + dec_in_flight_req(fsvq);
> + continue;
> + }
[Severity: Critical]
Can this still result in a DMA use-after-free on struct fuse_req and
ap->folios before virtqueue_get_buf() even returns?
In virtio_fs_enqueue_req(), the scatter-gather list passed to
virtqueue_add_sgs() maps &req->in.h, req->argbuf, and input ap->folios as
device-readable buffers, and maps &req->out.h (embedded inside struct
fuse_req), req->argbuf + argbuf_used, and output ap->folios as
device-writable buffers:
fs/fuse/virtio_fs.c:virtio_fs_enqueue_req() {
...
/* Request elements */
sg_init_one(&sg[out_sgs++], &req->in.h, sizeof(req->in.h));
out_sgs += sg_init_fuse_args(&sg[out_sgs], req,
(struct fuse_arg *)args->in_args,
args->in_numargs, args->in_pages,
req->argbuf, &argbuf_used);
/* Reply elements */
if (test_bit(FR_ISREPLY, &req->flags)) {
sg_init_one(&sg[out_sgs + in_sgs++],
&req->out.h, sizeof(req->out.h));
in_sgs += sg_init_fuse_args(&sg[out_sgs + in_sgs], req,
args->out_args, args->out_numargs,
args->out_pages,
req->argbuf + argbuf_used, NULL);
}
...
}
When fuse_chan_abort() runs while a request is in flight in the virtqueue,
virtio_fs_abort() sets fsvq->aborted = true without resetting the virtio
device, and fuse_chan_abort() immediately splices fpq->processing onto
to_end and calls fuse_dev_end_requests() -> fuse_request_end(). That invokes
req->args->end() (unlocking and releasing ap->folios) and frees struct
fuse_req back to fuse_req_cachep via fuse_request_free() while the
descriptor chain remains active in the virtqueue.
When the host later completes the stalled descriptor, won't the device (and
any DMA unmapping in virtqueue_get_buf()) write into &req->out.h inside the
already-freed struct fuse_req and into the released ap->folios before
reaching the fsvq->aborted check here?
[Severity: Medium]
Does skipping virtio_fs_request_complete() here also leak req->argbuf for
every aborted in-flight request?
When virtio_fs_enqueue_req() prepares a request, copy_args_to_argbuf()
allocates req->argbuf:
fs/fuse/virtio_fs.c:copy_args_to_argbuf() {
...
req->argbuf = kmalloc(len, gfp);
if (!req->argbuf)
return -ENOMEM;
...
}
Normally req->argbuf is freed in copy_args_from_argbuf() called from
virtio_fs_request_complete(). When fuse_chan_abort() ends requests on
fpq->processing via fuse_dev_end_requests() -> fuse_request_end() ->
fuse_put_request() -> fuse_request_free(), struct fuse_req is freed to
fuse_req_cachep without freeing req->argbuf:
fs/fuse/dev.c:fuse_request_free() {
WARN_ON(!list_empty(&req->intr_entry));
kmem_cache_free(fuse_req_cachep, req);
}
Since virtio_fs_requests_done_work() skips virtio_fs_request_complete() when
fsvq->aborted is true (and req is already freed at that point), where is
req->argbuf freed for aborted requests?
> if (!virtio_fs_verify_response(req, len)) {
> req->out.h.error = -EIO;
> req->out.h.len = sizeof(struct fuse_out_header);
> @@ -1570,10 +1580,26 @@ static void virtio_fs_send_req(struct fuse_iqueue *fiq, struct fuse_req *req)
> }
> }
>
> +static void virtio_fs_abort(struct fuse_iqueue *fiq)
> +{
> + struct virtio_fs *fs = fiq->priv;
> + int i;
> +
> + for (i = 0; i < fs->nvqs; i++) {
> + struct virtio_fs_vq *fsvq = &fs->vqs[i];
> +
> + spin_lock(&fsvq->lock);
> + fsvq->connected = false;
> + fsvq->aborted = true;
> + spin_unlock(&fsvq->lock);
> + }
> +}
[Severity: High]
Will setting fsvq->aborted = true here without clearing it when queues are
restarted break subsequent mounts of the same virtio-fs filesystem?
A struct virtio_fs instance and its fs->vqs[] array persist across mounts
for the lifetime of the virtio_device. On a normal unmount,
virtio_kill_sb() -> virtio_fs_conn_destroy() -> fuse_conn_destroy() calls
fuse_chan_abort(fc->chan, false):
fs/fuse/inode.c:fuse_conn_destroy() {
...
fuse_chan_abort(fc->chan, false);
fuse_chan_wait_aborted(fc->chan);
...
}
Because fch->connected is still set during a normal unmount,
fuse_chan_abort() invokes fiq->ops->abort(fiq) -> virtio_fs_abort(fiq),
which sets fsvq->aborted = true on every queue in fs->vqs[].
When the same virtio-fs tag is mounted again, virtio_fs_get_tree() ->
virtio_fs_fill_super() calls virtio_fs_start_all_queues(fs), which sets
fsvq->connected = true but never clears fsvq->aborted:
fs/fuse/virtio_fs.c:virtio_fs_start_all_queues() {
struct virtio_fs_vq *fsvq;
int i;
for (i = 0; i < fs->nvqs; i++) {
fsvq = &fs->vqs[i];
spin_lock(&fsvq->lock);
fsvq->connected = true;
spin_unlock(&fsvq->lock);
}
}
On the new mount, won't virtio_fs_requests_done_work() see fsvq->aborted ==
true and drop all completions (starting with FUSE_INIT from
fuse_send_init()), leaking req->argbuf and hanging the mount?
[Severity: High]
What happens to requests on fsvq->queued_reqs when virtio_fs_abort() runs?
When a virtqueue is full, virtio_fs_enqueue_req() drops fsvq->lock and
returns -ENOSPC without adding req to fpq->processing. Both
virtio_fs_send_req() and virtio_fs_request_dispatch_work() then re-acquire
fsvq->lock and place req onto fsvq->queued_reqs without checking
fsvq->connected:
fs/fuse/virtio_fs.c:virtio_fs_send_req() {
...
fsvq = &fs->vqs[queue_id];
ret = virtio_fs_enqueue_req(fsvq, req, false, GFP_ATOMIC);
if (ret < 0) {
if (ret == -ENOSPC) {
/*
* Virtqueue full. Retry submission from worker
* context as we might be holding fc->chan->bg_lock.
*/
spin_lock(&fsvq->lock);
list_add_tail(&req->list, &fsvq->queued_reqs);
inc_in_flight_req(fsvq);
spin_unlock(&fsvq->lock);
return;
}
...
}
Because FR_PENDING was already cleared in virtio_fs_send_req() and req is
not on fpq->processing, fuse_chan_abort() will not find these requests. In
addition, a racing virtio_fs_send_req() that saw fsvq->connected == true
inside virtio_fs_enqueue_req() and got -ENOSPC can acquire fsvq->lock after
virtio_fs_abort() has finished and still add req to fsvq->queued_reqs.
Since virtio_fs_abort() does not drain fsvq->queued_reqs or schedule
fsvq->dispatch_work (which is otherwise only scheduled from
virtio_fs_requests_done_work() when completions arrive), won't requests left
on fsvq->queued_reqs when the host stalls never be ended, hanging tasks in
request_wait_answer(), fuse_chan_wait_aborted(), and
virtio_fs_drain_queue()?
[ ... ]
விரும்பியபடி, இيمெயில் பதிலை மேலே உருவாக்கியுள்ளேன்.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261020135350.7751-1-q.h.hack.winter@gmail.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread