[PATCH 0/3] virtiofs: fix sashiko reported bugs

[PATCH 0/3] virtiofs: fix sashiko reported bugs

[Miklos Szeredi]

---
Miklos Szeredi (3):
  virtiofs: fix use-after-free of fs->tag on device removal
  virtiofs: disable request timeout
  virtiofs: zero the reply region of the argument buffer

 fs/fuse/fuse_i.h    | 10 ++++++++++
 fs/fuse/inode.c     |  9 ++++++---
 fs/fuse/virtio_fs.c | 33 +++++++++++++++++++++++++++------
 3 files changed, 43 insertions(+), 9 deletions(-)

-- 
2.56.0

[PATCH 1/3] virtiofs: fix use-after-free of fs->tag on device removal

[Miklos Szeredi]

The virtio_fs object is refcounted via its kobject, so a mount can keep it
alive well beyond the lifetime of the virtio device binding.  The tag
string, however, was allocated with devm_kmalloc() and is therefore freed
by devres_release_all() as soon as the device is unbound, i.e. right after
virtio_fs_remove() returns (see device_unbind_cleanup() in
drivers/base/dd.c).

virtio_fs_get_tree() looks up the instance, takes a kobject reference and
then drops virtio_fs_mutex before doing the sleepable part of the mount
(allocations, fuse_conn_init(), sget_fc()).  A concurrent device removal
can complete in that window:

  CPU 0 (mount)                         CPU 1 (remove)
  -------------                         --------------
  virtio_fs_get_tree()
    virtio_fs_find_instance()
    // reference taken, mutex dropped
                                        virtio_fs_remove()
                                          list_del_init(&fs->list);
                                        // driver core unbinds device
                                        // devres frees fs->tag
  virtio_fs_fill_super()
    mutex_lock(&virtio_fs_mutex);
    if (list_empty(&fs->list))
            pr_info(... fs->tag);   <-- use-after-free

The virtio_fs object itself is still valid here, only its tag has been
freed behind its back.

Fix the lifetime mismatch at the source: allocate the tag with kmalloc()
and free it from virtio_fs_ktype_release(), so that it lives exactly as
long as the object embedding it.  All probe error paths already drop the
final kobject reference, so they are covered.  Merely dropping the tag
from the error message would paper over this instance while leaving a
devres pointer embedded in an object that routinely outlives its device.

The other readers of fs->tag are unaffected: tag_show() is fenced off by
the kobject_del() in virtio_fs_remove(), the automatic KOBJ_REMOVE uevent
is emitted from __kobject_del() while the device is still bound, and both
virtio_fs_add_instance() and virtio_fs_find_instance() only walk instances
still on virtio_fs_instances.

Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
Cc: <stable@vger.kernel.org>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-0328b3a1-043b-468e-8425-6a613d91b5ef
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/virtio_fs.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index ba8d79c4bcac..ef58bedc3941 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -193,6 +193,7 @@ static void virtio_fs_ktype_release(struct kobject *kobj)
 {
 	struct virtio_fs *vfs = container_of(kobj, struct virtio_fs, kobj);
 
+	kfree(vfs->tag);
 	kfree(vfs->mq_map);
 	kfree(vfs->vqs);
 	kfree(vfs);
@@ -489,7 +490,11 @@ static void virtio_fs_free_devs(struct virtio_fs *fs)
 	}
 }
 
-/* Read filesystem name from virtio config into fs->tag (must kfree()). */
+/*
+ * Read filesystem name from virtio config into fs->tag.  Freed by
+ * virtio_fs_ktype_release() so that it stays valid for as long as the
+ * virtio_fs object itself, which may outlive the virtio device.
+ */
 static int virtio_fs_read_tag(struct virtio_device *vdev, struct virtio_fs *fs)
 {
 	char tag_buf[sizeof_field(struct virtio_fs_config, tag)];
@@ -505,7 +510,7 @@ static int virtio_fs_read_tag(struct virtio_device *vdev, struct virtio_fs *fs)
 		end = &tag_buf[sizeof(tag_buf)];
 
 	len = end - tag_buf;
-	fs->tag = devm_kmalloc(&vdev->dev, len + 1, GFP_KERNEL);
+	fs->tag = kmalloc(len + 1, GFP_KERNEL);
 	if (!fs->tag)
 		return -ENOMEM;
 	memcpy(fs->tag, tag_buf, len);
-- 
2.56.0

Re: [PATCH 1/3] virtiofs: fix use-after-free of fs->tag on device removal

[sashiko-bot]

> The virtio_fs object is refcounted via its kobject, so a mount can keep it
> alive well beyond the lifetime of the virtio device binding.  The tag
> string, however, was allocated with devm_kmalloc() and is therefore freed
> by devres_release_all() as soon as the device is unbound, i.e. right after
> virtio_fs_remove() returns (see device_unbind_cleanup() in
> drivers/base/dd.c).
> 
> virtio_fs_get_tree() looks up the instance, takes a kobject reference and
> then drops virtio_fs_mutex before doing the sleepable part of the mount
> (allocations, fuse_conn_init(), sget_fc()).  A concurrent device removal
> can complete in that window:
> 
>   CPU 0 (mount)                         CPU 1 (remove)
>   -------------                         --------------
>   virtio_fs_get_tree()
> [ ... ]
> Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
> Cc: <stable@vger.kernel.org>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/bug/linux-0328b3a1-043b-468e-8425-6a613d91b5ef
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105150943.86650-1-mszeredi@redhat.com?part=1

Re: [PATCH 1/3] virtiofs: fix use-after-free of fs->tag on device removal

[Li Wang]

Hi Miklos,

On 05/11/2026 23:09, Miklos Szeredi wrote:
> The virtio_fs object is refcounted via its kobject, so a mount can keep it
> alive well beyond the lifetime of the virtio device binding.  The tag
> string, however, was allocated with devm_kmalloc() and is therefore freed
> by devres_release_all() as soon as the device is unbound, i.e. right after
> virtio_fs_remove() returns (see device_unbind_cleanup() in
> drivers/base/dd.c).
> 
> virtio_fs_get_tree() looks up the instance, takes a kobject reference and
> then drops virtio_fs_mutex before doing the sleepable part of the mount
> (allocations, fuse_conn_init(), sget_fc()).  A concurrent device removal
> can complete in that window:
> 
>   CPU 0 (mount)                         CPU 1 (remove)
>   -------------                         --------------
>   virtio_fs_get_tree()
>     virtio_fs_find_instance()
>     // reference taken, mutex dropped
>                                         virtio_fs_remove()
>                                           list_del_init(&fs->list);
>                                         // driver core unbinds device
>                                         // devres frees fs->tag
>   virtio_fs_fill_super()
>     mutex_lock(&virtio_fs_mutex);
>     if (list_empty(&fs->list))
>             pr_info(... fs->tag);   <-- use-after-free
> 
> The virtio_fs object itself is still valid here, only its tag has been
> freed behind its back.
> 
> Fix the lifetime mismatch at the source: allocate the tag with kmalloc()
> and free it from virtio_fs_ktype_release(), so that it lives exactly as
> long as the object embedding it.  All probe error paths already drop the
> final kobject reference, so they are covered.  Merely dropping the tag
> from the error message would paper over this instance while leaving a
> devres pointer embedded in an object that routinely outlives its device.
> 
> The other readers of fs->tag are unaffected: tag_show() is fenced off by
> the kobject_del() in virtio_fs_remove(), the automatic KOBJ_REMOVE uevent
> is emitted from __kobject_del() while the device is still bound, and both
> virtio_fs_add_instance() and virtio_fs_find_instance() only walk instances
> still on virtio_fs_instances.
> 
> Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
> Cc: <stable@vger.kernel.org>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/bug/linux-0328b3a1-043b-468e-8425-6a613d91b5ef
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/virtio_fs.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index ba8d79c4bcac..ef58bedc3941 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> ...
> -/* Read filesystem name from virtio config into fs->tag (must kfree()). */
> +/*
> + * Read filesystem name from virtio config into fs->tag.  Freed by
> + * virtio_fs_ktype_release() so that it stays valid for as long as the
> + * virtio_fs object itself, which may outlive the virtio device.
> + */
>  static int virtio_fs_read_tag(struct virtio_device *vdev, struct virtio_fs *fs)
>  {
>  	char tag_buf[sizeof_field(struct virtio_fs_config, tag)];
> @@ -505,7 +510,7 @@ static int virtio_fs_read_tag(struct virtio_device *vdev, struct virtio_fs *fs)
>  		end = &tag_buf[sizeof(tag_buf)];
>  
>  	len = end - tag_buf;
> -	fs->tag = devm_kmalloc(&vdev->dev, len + 1, GFP_KERNEL);
> +	fs->tag = kmalloc(len + 1, GFP_KERNEL);

The fix for the UAF issue regarding fs->tag looks good to me.

However, while looking at this, I happened to notice another similar potential issue: when the DAX mode 
is enabled for file read/write, the DAX window is mapped via devm_memremap_pages and unmapped when the device 
is unbound. Although as far as I know, the corresponding QEMU changes for DAX mode don't seem to have landed 
in QEMU upstream, fs->window_kaddr and other established page table mappings still point to this memory region.
Perhaps we should also look into cleaning them up?

Reviewed-by: Li Wang <liwang@kylinos.cn>

[PATCH 2/3] virtiofs: disable request timeout

[Miklos Szeredi]

The request timeout is implemented by aborting the connection:
fuse_check_timeout() calls fuse_chan_abort(), which splices
fpq->processing onto a local list and ends those requests.  Ending a
request wakes the waiting caller, which then drops the last reference and
frees it.

That is safe for /dev/fuse, where the request has already been copied out
to userspace and nothing outside the kernel retains a pointer to it.  It
is not safe for virtiofs.  virtio_fs_enqueue_req() passes the bare req as
the virtqueue token, and the scatterlists point at req->in.h, req->out.h,
req->argbuf and the request's folios; the device still owns all of that
after the abort.  When the device eventually completes the request,
virtio_fs_requests_done_work() gets the freed req back from
virtqueue_get_buf() and passes it to virtio_fs_verify_response() and
virtio_fs_request_complete(), which dereference req->args - the caller's
stack frame, long gone - and kfree() whatever req->argbuf now reads as.
The device may also have written the reply into the freed request or into
folios that have since been reused.

This is precisely why virtio_fs_conn_destroy() drains all queues down to
in_flight == 0 before calling fuse_conn_destroy(): once buffers are in a
virtqueue, the only ways to reclaim them are to wait for completion or to
reset the device.  virtiofs already disables the other two paths that
could abort a live connection, via ctx->no_control and
ctx->no_force_umount; the timeout is the one that was left.

Timeouts make little sense for virtiofs in the first place.  A timeout is
a guest-side reaction to a wedged host, and the guest has no recourse: it
can neither take the buffers back nor make the host answer.

So add fuse_conn::no_req_timeout for transports that cannot revoke a
request once it has been handed to the server, set it from virtiofs, and
use it to skip both fuse_init_server_timeout() and the
FUSE_REQUEST_TIMEOUT negotiation flag.  Skipping the former matters on its
own: without it the fs/fuse/default_request_timeout sysctl would arm the
timer even though FUSE_REQUEST_TIMEOUT was never offered to the server.

Note that this is not a security issue.  virtiofs inverts the usual fuse
trust relationship - the server is the privileged, trusted side and the
guest kernel is the untrusted one - so a server intent on corrupting guest
memory has far more direct means at its disposal.

Fixes: 0f6439f61a6e ("fuse: add kernel-enforced timeout option for requests")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-8171ebf0-ef01-4ccd-943e-8633a8d54239
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/fuse_i.h    | 10 ++++++++++
 fs/fuse/inode.c     |  9 ++++++---
 fs/fuse/virtio_fs.c |  7 +++++++
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index ec5ccdf18a25..a033b0bc4576 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -418,6 +418,7 @@ struct fuse_fs_context {
 	bool destroy:1;
 	bool no_control:1;
 	bool no_force_umount:1;
+	bool no_req_timeout:1;
 	bool legacy_opts_show:1;
 	bool syncfs_capable:1;
 	enum fuse_vdax_mode vdax_mode;
@@ -698,6 +699,15 @@ struct fuse_conn {
 	/** @no_force_umount: Do not allow MNT_FORCE umount */
 	unsigned int no_force_umount:1;
 
+	/**
+	 * @no_req_timeout: Do not enforce a request timeout
+	 *
+	 * Set by transports that cannot revoke a request once it has been
+	 * handed to the server, since the timeout is implemented by aborting
+	 * the connection.
+	 */
+	unsigned int no_req_timeout:1;
+
 	/** @auto_submounts: Auto-mount submounts announced by the server */
 	unsigned int auto_submounts:1;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 1341b681d0ed..3c0a629cc2fe 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1451,7 +1451,8 @@ static void process_init_reply(struct fuse_args *args, int error)
 			fc->no_flock = 1;
 		}
 
-		fuse_init_server_timeout(fc->chan, timeout);
+		if (!fc->no_req_timeout)
+			fuse_init_server_timeout(fc->chan, timeout);
 
 		fm->sb->s_bdi->ra_pages =
 				min(fm->sb->s_bdi->ra_pages, ra_pages);
@@ -1502,8 +1503,9 @@ static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
 		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
 		FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP |
 		FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP |
-		FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND | FUSE_ALLOW_IDMAP |
-		FUSE_REQUEST_TIMEOUT;
+		FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND | FUSE_ALLOW_IDMAP;
+	if (!fm->fc->no_req_timeout)
+		flags |= FUSE_REQUEST_TIMEOUT;
 #ifdef CONFIG_FUSE_VDAX
 	if (fm->fc->vdax)
 		flags |= FUSE_MAP_ALIGNMENT;
@@ -1820,6 +1822,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 	fc->destroy = ctx->destroy;
 	fc->no_control = ctx->no_control;
 	fc->no_force_umount = ctx->no_force_umount;
+	fc->no_req_timeout = ctx->no_req_timeout;
 
 	err = -ENOMEM;
 	root = fuse_get_root_inode(sb, ctx->rootmode);
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index ef58bedc3941..c824916f0a9b 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1592,6 +1592,13 @@ static inline void virtio_fs_ctx_set_defaults(struct fuse_fs_context *ctx)
 	ctx->destroy = true;
 	ctx->no_control = true;
 	ctx->no_force_umount = true;
+	/*
+	 * A request handed to the device stays referenced by the virtqueue
+	 * until the device completes it.  The request timeout works by
+	 * aborting the connection, which ends and frees such requests behind
+	 * the device's back, so it must not be enabled here.
+	 */
+	ctx->no_req_timeout = true;
 }
 
 static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
-- 
2.56.0

Re: [PATCH 2/3] virtiofs: disable request timeout

[sashiko-bot]

> The request timeout is implemented by aborting the connection:
> fuse_check_timeout() calls fuse_chan_abort(), which splices
> fpq->processing onto a local list and ends those requests.  Ending a
> request wakes the waiting caller, which then drops the last reference and
> frees it.
> 
> That is safe for /dev/fuse, where the request has already been copied out
> to userspace and nothing outside the kernel retains a pointer to it.  It
> is not safe for virtiofs.  virtio_fs_enqueue_req() passes the bare req as
> the virtqueue token, and the scatterlists point at req->in.h, req->out.h,
> req->argbuf and the request's folios; the device still owns all of that
> after the abort.  When the device eventually completes the request,
> virtio_fs_requests_done_work() gets the freed req back from
> virtqueue_get_buf() and passes it to virtio_fs_verify_response() and
> virtio_fs_request_complete(), which dereference req->args - the caller's
> [ ... ]
> 
> Fixes: 0f6439f61a6e ("fuse: add kernel-enforced timeout option for requests")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/bug/linux-8171ebf0-ef01-4ccd-943e-8633a8d54239
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105150943.86650-1-mszeredi@redhat.com?part=2

Re: [PATCH 2/3] virtiofs: disable request timeout

[Li Wang]

Hi Miklos,

On 05/11/2026 23:09, Miklos Szeredi wrote:
> The request timeout is implemented by aborting the connection:
> fuse_check_timeout() calls fuse_chan_abort(), which splices
> fpq->processing onto a local list and ends those requests.  Ending a
> request wakes the waiting caller, which then drops the last reference and
> frees it.
> 
> That is safe for /dev/fuse, where the request has already been copied out
> to userspace and nothing outside the kernel retains a pointer to it.  It
> is not safe for virtiofs.  virtio_fs_enqueue_req() passes the bare req as
> the virtqueue token, and the scatterlists point at req->in.h, req->out.h,
> req->argbuf and the request's folios; the device still owns all of that
> after the abort.  When the device eventually completes the request,
> virtio_fs_requests_done_work() gets the freed req back from
> virtqueue_get_buf() and passes it to virtio_fs_verify_response() and
> virtio_fs_request_complete(), which dereference req->args - the caller's
> stack frame, long gone - and kfree() whatever req->argbuf now reads as.
> The device may also have written the reply into the freed request or into
> folios that have since been reused.
> 
> This is precisely why virtio_fs_conn_destroy() drains all queues down to
> in_flight == 0 before calling fuse_conn_destroy(): once buffers are in a
> virtqueue, the only ways to reclaim them are to wait for completion or to
> reset the device.  virtiofs already disables the other two paths that
> could abort a live connection, via ctx->no_control and
> ctx->no_force_umount; the timeout is the one that was left.
> 
> Timeouts make little sense for virtiofs in the first place.  A timeout is
> a guest-side reaction to a wedged host, and the guest has no recourse: it
> can neither take the buffers back nor make the host answer.
> 
> So add fuse_conn::no_req_timeout for transports that cannot revoke a
> request once it has been handed to the server, set it from virtiofs, and
> use it to skip both fuse_init_server_timeout() and the
> FUSE_REQUEST_TIMEOUT negotiation flag.  Skipping the former matters on its
> own: without it the fs/fuse/default_request_timeout sysctl would arm the
> timer even though FUSE_REQUEST_TIMEOUT was never offered to the server.
> 
> Note that this is not a security issue.  virtiofs inverts the usual fuse
> trust relationship - the server is the privileged, trusted side and the
> guest kernel is the untrusted one - so a server intent on corrupting guest
> memory has far more direct means at its disposal.
> 
> Fixes: 0f6439f61a6e ("fuse: add kernel-enforced timeout option for requests")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/bug/linux-8171ebf0-ef01-4ccd-943e-8633a8d54239
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/fuse_i.h    | 10 ++++++++++
>  fs/fuse/inode.c     |  9 ++++++---
>  fs/fuse/virtio_fs.c |  7 +++++++
>  3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index ec5ccdf18a25..a033b0bc4576 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -418,6 +418,7 @@ struct fuse_fs_context {
>  	bool destroy:1;
>  	bool no_control:1;
>  	bool no_force_umount:1;
> +	bool no_req_timeout:1;
>  	bool legacy_opts_show:1;
>  	bool syncfs_capable:1;
>  	enum fuse_vdax_mode vdax_mode;
> @@ -698,6 +699,15 @@ struct fuse_conn {
>  	/** @no_force_umount: Do not allow MNT_FORCE umount */
>  	unsigned int no_force_umount:1;
>  
> +	/**
> +	 * @no_req_timeout: Do not enforce a request timeout
> +	 *
> +	 * Set by transports that cannot revoke a request once it has been
> +	 * handed to the server, since the timeout is implemented by aborting
> +	 * the connection.
> +	 */
> +	unsigned int no_req_timeout:1;
> +

First, I'm wondering if we should also update the description of the FUSE timeout 
files in Documentation/admin-guide/sysctl/fs.rst.

Additionally, I have a further thought regarding the current virtiofs implementation: 
if the vhost-user backend (such as virtiofsd) hangs or exits unexpectedly, guest 
applications accessing virtiofs will hang indefinitely in the guest kernel. The dirty 
pages currently under writeback continue to hold the writeback state, preventing the 
filesystem from being unmounted. Furthermore, when writeback or page reclaim subsequently 
encounters these pages, the hang can propagate to other processes in the guest. Even restarting 
virtiofsd on the host does not resolve this, which can be quite unfriendly to users. Given this, 
I'm wondering if we should consider introducing a fallback or recovery mechanism for 
virtiofs—perhaps allowing a manual write to a file under /sys/fs/virtiofs/<device_id> or permitting 
a forced unmount to invoke virtio_reset_device() for cleanup. What are your thoughts on this?

Thanks,
Li Wang

[PATCH 3/3] virtiofs: zero the reply region of the argument buffer

[Miklos Szeredi]

copy_args_to_argbuf() allocates one buffer for the fixed in- and out-args
and fills only the in-arg part.  The remainder is handed to the server as
a device-writable buffer, and the server is under no obligation to write
it: the used length it reports is its own claim and
virtio_fs_verify_response() has nothing to cross-check it against.

A reply that declares its full length without filling the buffer makes
copy_args_from_argbuf() copy the uninitialized remainder to the caller,
so a server that fails partway through writing a reply returns stale heap
contents from e.g. fuse_statfs() or fuse_getxattr() instead of an error.

Commit 5364d081125a ("virtiofs: validate fixed-output response length")
covers the case where the declared length is short.  Zero the reply
region so that the remaining case yields zeroes rather than whatever the
allocator last held.  Only that region is zeroed; the in-arg part is
overwritten by the copy below and the buffer can be as large as
XATTR_SIZE_MAX.

Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-f0d6d4a3-f5df-4f72-ab09-802e5aab49a6
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/virtio_fs.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index c824916f0a9b..cdd6f5c50d6d 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -701,15 +701,16 @@ static int copy_args_to_argbuf(struct fuse_req *req, gfp_t gfp)
 	unsigned int offset = 0;
 	unsigned int num_in;
 	unsigned int num_out;
-	unsigned int len;
+	unsigned int in_len;
+	unsigned int out_len;
 	unsigned int i;
 
 	num_in = args->in_numargs - args->in_pages;
 	num_out = args->out_numargs - args->out_pages;
-	len = fuse_len_args(num_in, (struct fuse_arg *) args->in_args) +
-	      fuse_len_args(num_out, args->out_args);
+	in_len = fuse_len_args(num_in, (struct fuse_arg *) args->in_args);
+	out_len = fuse_len_args(num_out, args->out_args);
 
-	req->argbuf = kmalloc(len, gfp);
+	req->argbuf = kmalloc(in_len + out_len, gfp);
 	if (!req->argbuf)
 		return -ENOMEM;
 
@@ -720,6 +721,14 @@ static int copy_args_to_argbuf(struct fuse_req *req, gfp_t gfp)
 		offset += args->in_args[i].size;
 	}
 
+	/*
+	 * The server is not required to fill the reply region and the length
+	 * it reports in the used ring cannot confirm that it did.  Zero the
+	 * region so that an unwritten reply yields zeroes rather than stale
+	 * heap contents.
+	 */
+	memset(req->argbuf + in_len, 0, out_len);
+
 	return 0;
 }
 
-- 
2.56.0

Re: [PATCH 3/3] virtiofs: zero the reply region of the argument buffer

[sashiko-bot]

> copy_args_to_argbuf() allocates one buffer for the fixed in- and out-args
> and fills only the in-arg part.  The remainder is handed to the server as
> a device-writable buffer, and the server is under no obligation to write
> it: the used length it reports is its own claim and
> virtio_fs_verify_response() has nothing to cross-check it against.
> 
> A reply that declares its full length without filling the buffer makes
> copy_args_from_argbuf() copy the uninitialized remainder to the caller,
> so a server that fails partway through writing a reply returns stale heap
> contents from e.g. fuse_statfs() or fuse_getxattr() instead of an error.
> 
> Commit 5364d081125a ("virtiofs: validate fixed-output response length")
> covers the case where the declared length is short.  Zero the reply
> region so that the remaining case yields zeroes rather than whatever the
> allocator last held.  Only that region is zeroed; the in-arg part is
> overwritten by the copy below and the buffer can be as large as
> XATTR_SIZE_MAX.
> 
> Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/bug/linux-f0d6d4a3-f5df-4f72-ab09-802e5aab49a6
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105150943.86650-1-mszeredi@redhat.com?part=3

Re: [PATCH 3/3] virtiofs: zero the reply region of the argument buffer

[Li Wang]

Hi Miklos,

On 05/11/2026 23:09, Miklos Szeredi wrote:
> copy_args_to_argbuf() allocates one buffer for the fixed in- and out-args
> and fills only the in-arg part.  The remainder is handed to the server as
> a device-writable buffer, and the server is under no obligation to write
> it: the used length it reports is its own claim and
> virtio_fs_verify_response() has nothing to cross-check it against.
> 
> A reply that declares its full length without filling the buffer makes
> copy_args_from_argbuf() copy the uninitialized remainder to the caller,
> so a server that fails partway through writing a reply returns stale heap
> contents from e.g. fuse_statfs() or fuse_getxattr() instead of an error.
> 
> Commit 5364d081125a ("virtiofs: validate fixed-output response length")
> covers the case where the declared length is short.  Zero the reply
> region so that the remaining case yields zeroes rather than whatever the
> allocator last held.  Only that region is zeroed; the in-arg part is
> overwritten by the copy below and the buffer can be as large as
> XATTR_SIZE_MAX.
> 
> Fixes: a62a8ef9d97d ("virtio-fs: add virtiofs filesystem")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/bug/linux-f0d6d4a3-f5df-4f72-ab09-802e5aab49a6
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/virtio_fs.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index c824916f0a9b..cdd6f5c50d6d 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -701,15 +701,16 @@ static int copy_args_to_argbuf(struct fuse_req *req, gfp_t gfp)
> ...
>  
> +	/*
> +	 * The server is not required to fill the reply region and the length
> +	 * it reports in the used ring cannot confirm that it did.  Zero the
> +	 * region so that an unwritten reply yields zeroes rather than stale
> +	 * heap contents.
> +	 */
> +	memset(req->argbuf + in_len, 0, out_len);
> +

For the virtiofs scenario, the server (i.e., host) is trusted. Ideally, the host 
should not write a size without completing the data—or at least not maliciously so 
(at most a bug, which should be fixed on the host side, whereas if it were malicious, 
we couldn't defend against it anyway). Since QEMU/virtiofsd can access all guest memory, 
a malicious host could always leak guest kernel memory contents through channels like 
fuse_statfs responses. Given this, I'm wondering if adding this memset is worthwhile.

Thanks,
Li Wang