On 01.04.25 15:44, Eric Blake wrote:
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?
That would be news to me, but if so, I’ll be happy to use it. (The
problem is distinguishing between what needs qemu_vfree(), and what
needs g_free(), I suppose.)
Hanna
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?
