Re: [PATCH] migration/file: fix type mismatch and NULL deref in multifd_file_recv_data

[Peter Xu <peterx@redhat.com>]

On Mon, Mar 16, 2026 at 04:46:18PM +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 changing ret to ssize_t and splitting the error
> handling: use error_prepend() only when qio_channel_pread() itself
> has populated *errp (ret < 0), and error_setg() for the short-read
> case where *errp has not been set.  Add ERRP_GUARD() so that
> error_prepend() works correctly even when errp is &error_fatal or
> NULL.

Indeed, this seems problematic.

But is it possible to get partial reads?  I don't see why it won't happen..
do we need to fix it too (e.g. introduce qio_channel_pread_all_eof())?

CC Dan.

Thanks,

> 
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
> ---
>  migration/file.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/file.c b/migration/file.c
> index 5618aced49..78b274dc32 100644
> --- a/migration/file.c
> +++ b/migration/file.c
> @@ -254,15 +254,21 @@ 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;
> +    ssize_t ret;
>  
>      ret = qio_channel_pread(p->c, (char *) data->opaque,
>                              data->size, data->file_offset, errp);
> +    if (ret < 0) {
> +        error_prepend(errp, "multifd recv (%u): ", p->id);
> +        return -1;
> +    }
> +
>      if (ret != data->size) {
> -        error_prepend(errp,
> -                      "multifd recv (%u): read 0x%zx, expected 0x%zx",
> -                      p->id, ret, data->size);
> +        error_setg(errp,
> +                   "multifd recv (%u): read 0x%zx, expected 0x%zx",
> +                   p->id, (size_t)ret, data->size);
>          return -1;
>      }
>  
> -- 
> 2.43.0
> 

-- 
Peter Xu