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 <[email protected]>
Closes: https://sashiko.dev/#/bug/linux-8171ebf0-ef01-4ccd-943e-8633a8d54239
Signed-off-by: Miklos Szeredi <[email protected]>
---
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
