[PATCH 2/3] virtiofs: disable request timeout

[Miklos Szeredi <mszeredi@redhat.com>]

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