[PATCH v2 2/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data

Junjie Cao posted 3 patches 2 weeks, 5 days ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
[PATCH v2 2/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data
Posted by Junjie Cao 2 weeks, 5 days ago
multifd_file_recv_data() stores the return value of qio_channel_pread()
(ssize_t) in a size_t variable.  On I/O error the -1 return value wraps
to SIZE_MAX, producing a nonsensical read size in the error message.

More critically, a short read (0 <= ret < data->size) is possible when
the migration file is truncated.  In that case qio_channel_pread()
returns a non-negative value without setting *errp.  The function then
calls error_prepend(errp, ...) which dereferences *errp -- a NULL
pointer -- crashing QEMU.

Fix both issues by switching to qio_channel_pread_all() introduced in
the previous commit, which retries on short reads and treats
end-of-file as an error, so the caller no longer needs to check the
byte count manually.  Add ERRP_GUARD() so that error_prepend() works
correctly even when errp is &error_fatal or NULL.

Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
 migration/file.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/migration/file.c b/migration/file.c
index 5618aced49..b0d3affa4c 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -254,16 +254,15 @@ int file_write_ramblock_iov(QIOChannel *ioc, const struct iovec *iov,
 
 int multifd_file_recv_data(MultiFDRecvParams *p, Error **errp)
 {
+    ERRP_GUARD();
     MultiFDRecvData *data = p->data;
-    size_t ret;
-
-    ret = qio_channel_pread(p->c, (char *) data->opaque,
-                            data->size, data->file_offset, errp);
-    if (ret != data->size) {
-        error_prepend(errp,
-                      "multifd recv (%u): read 0x%zx, expected 0x%zx",
-                      p->id, ret, data->size);
-        return -1;
+    int ret;
+
+    ret = qio_channel_pread_all(p->c, (char *) data->opaque,
+                                data->size, data->file_offset, errp);
+    if (ret < 0) {
+        error_prepend(errp, "multifd recv (%u): ", p->id);
+        return ret;
     }
 
     return 0;
-- 
2.43.0
Re: [PATCH v2 2/3] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data
Posted by Daniel P. Berrangé 1 week, 6 days ago
On Wed, Mar 18, 2026 at 10:01:12PM +0800, Junjie Cao wrote:
> multifd_file_recv_data() stores the return value of qio_channel_pread()
> (ssize_t) in a size_t variable.  On I/O error the -1 return value wraps
> to SIZE_MAX, producing a nonsensical read size in the error message.
> 
> More critically, a short read (0 <= ret < data->size) is possible when
> the migration file is truncated.  In that case qio_channel_pread()
> returns a non-negative value without setting *errp.  The function then
> calls error_prepend(errp, ...) which dereferences *errp -- a NULL
> pointer -- crashing QEMU.
> 
> Fix both issues by switching to qio_channel_pread_all() introduced in
> the previous commit, which retries on short reads and treats
> end-of-file as an error, so the caller no longer needs to check the
> byte count manually.  Add ERRP_GUARD() so that error_prepend() works
> correctly even when errp is &error_fatal or NULL.
> 
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
> ---
>  migration/file.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|