On Tue, Mar 25, 2025 at 05:06:35PM +0100, Hanna Czenczek wrote:
> Polling in I/O functions can lead to nested read_from_fuse_export()
> calls, overwriting the request buffer's content. The only function
> affected by this is fuse_write(), which therefore must use a bounce
> buffer or corruption may occur.
>
> Note that in addition we do not know whether libfuse-internal structures
> can cope with this nesting, and even if we did, we probably cannot rely
> on it in the future. This is the main reason why we want to remove
> libfuse from the I/O path.
>
> @@ -624,6 +630,7 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode,
> const char *buf,
> size_t size, off_t offset, struct fuse_file_info *fi)
> {
> FuseExport *exp = fuse_req_userdata(req);
> + void *copied;
Do we have a good way to annotate variables that require qemu_vfree()
if non-NULL for automatic cleanup? If so, should this be annotated
and set to NULL here,...
> int64_t length;
> int ret;
>
> @@ -638,6 +645,14 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode,
> const char *buf,
> return;
> }
>
> + /*
> + * Heed the note on read_from_fuse_export(): If we poll (which any
> blk_*()
> + * I/O function may do), read_from_fuse_export() may be nested,
> overwriting
> + * the request buffer content. Therefore, we must copy it here.
> + */
> + copied = blk_blockalign(exp->common.blk, size);
> + memcpy(copied, buf, size);
> +
> /**
> * Clients will expect short writes at EOF, so we have to limit
> * offset+size to the image length.
> @@ -645,7 +660,7 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode,
> const char *buf,
> length = blk_getlength(exp->common.blk);
> if (length < 0) {
> fuse_reply_err(req, -length);
> - return;
> + goto free_buffer;
...so that this and similar hunks are not needed?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
