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 <[email protected]>
Closes: https://sashiko.dev/#/bug/linux-f0d6d4a3-f5df-4f72-ab09-802e5aab49a6
Signed-off-by: Miklos Szeredi <[email protected]>
---
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
