Re: [PATCH] util/error, hw/net/virtio-net: fix NULL pointer dereference in error_vprepend

["Michael S. Tsirkin" <mst@redhat.com>]

Thanks for the patch! Some questions:

On Tue, Sep 15, 2026 at 07:24:39PM +0300, Николай Зорин wrote:
> SWSA#b0709cee-9722-4603-a30c-dc857dc87961

what is this?

> The `error_vprepend()` function modifies the error message inside an
> existing Error object. However, it only validated if the `errp` pointer
> itself was non-NULL, missing a check for whether the underlying Error
> object `*errp` is `NULL`.
> 
> Calling `error_reportf_err(local_error, ...)` when `local_error` is `NULL`
> invokes `error_vprepend(&local_error, ...)`, which subsequently crashes
> via a `NULL` pointer dereference during `(*errp)->msg` access.

but you then go on to remove such calls? then why do we need to change this?

> 
> Fix `util/error.c` by adding a defensive `!*errp` check inside
> `error_vprepend()`.
> 
> Additionally, fix the invalid error reporting patterns in
> `hw/net/virtio-net.c`:
> - In `vhost_user_net_save_state()` and `vhost_user_net_load_state()`, when
>   `vhdev == NULL`,

and when is that?

> calling `error_reportf_err()` with an unallocated/NULL
>   `local_error` is incorrect and unsafe. Replace it with a straightforward
>   `error_report()` call.
> - Fix a cosmetic double-space typo in "Error loading  back-end state".
> 
> 

why two empty lines here?

Fixes tag please?

> Signed-off-by: Nikolay N Zorin <zorin@swemel.ru>
> ---
>  hw/net/virtio-net.c | 14 +++++++-------
>  util/error.c        |  2 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 4362e866..d505fb86 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3406,9 +3406,9 @@ static int vhost_user_net_save_state(QEMUFile *f, void
> *pv, size_t size,
> 
>      vhdev = virtio_net_get_vhost(vdev);
>      if (vhdev == NULL) {
> -        error_reportf_err(local_error,
> -                          "Error getting vhost back-end of %s device %s: ",
> -                          vdev->name, vdev->parent_obj.canonical_path);
> +        error_report("Error getting vhost back-end of %s device %s: "
> +                     "device is not using vhost\n",
> +                     vdev->name, vdev->parent_obj.canonical_path);
>          return -1;
>      }
> 
> @@ -3434,16 +3434,16 @@ static int vhost_user_net_load_state(QEMUFile *f,
> void *pv, size_t size,
> 
>      vhdev = virtio_net_get_vhost(vdev);
>      if (vhdev == NULL) {
> -        error_reportf_err(local_error,
> -                          "Error getting vhost back-end of %s device %s: ",
> -                          vdev->name, vdev->parent_obj.canonical_path);
> +        error_report("Error getting vhost back-end of %s device %s: "
> +                     "device is not using vhost\n",
> +                     vdev->name, vdev->parent_obj.canonical_path);
>          return -1;
>      }
> 
>      ret = vhost_load_backend_state(vhdev, f, &local_error);
>      if (ret < 0) {
>          error_reportf_err(local_error,
> -                          "Error loading  back-end state of %s device %s: ",
> +                          "Error loading back-end state of %s device %s: ",
>                            vdev->name, vdev->parent_obj.canonical_path);
>          return ret;
>      }
> diff --git a/util/error.c b/util/error.c
> index 673011b8..db8b5e43 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -133,7 +133,7 @@ void error_vprepend(Error *const *errp, const char *fmt,
> va_list ap)
>  {
>      GString *newmsg;
> 
> -    if (!errp) {
> +    if (!errp || !*errp) {
>          return;
>      }
> 
> -- 
> 2.43.0