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

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

[Николай Зорин]

SWSA#b0709cee-9722-4603-a30c-dc857dc87961

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.

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`, 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".


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

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

[Michael S. Tsirkin]

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

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

[Markus Armbruster]

First, please read docs/devel/code-provenance.rst section "Use of
AI-generated content".

I'm not looking at the patches, only the commit message.

Николай Зорин <zorin@swemel.ru> writes:

> SWSA#b0709cee-9722-4603-a30c-dc857dc87961
>
> 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`.

This is nonsense.  error_vprepend() and error_prepend() modify an error.
They must not be called when there is none.  The check isn't missing.
Adding it makes the functions "work" papers over bugs.

> 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.
>
> 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`, calling `error_reportf_err()` with an unallocated/NULL
>    `local_error` is incorrect and unsafe. Replace it with a straightforward
>    `error_report()` call.

Both uses of error_reportf_err() are indeed wrong.

> - Fix a cosmetic double-space typo in "Error loading  back-end state".

Worth cleaning up.

> Signed-off-by: Nikolay N Zorin <zorin@swemel.ru>

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

[Николай Зорин]

Hello!

This is not AI-generated content. We have a dedicated team of engineers 
who monitor and analyze security events (ranging from ASan/TSan 
sanitizers to PVS-Studio) and perform statistical analysis (from 
cppcheck to SVACE and white bear). This is a strict requirement from our 
Quality Assurance department, which is mandatory for compliance with our 
government and commercial contracts. Unfortunately, I cannot share our 
internal security tools (especially since they are tailored to our 
environment and won't work for you). However, I can share what our 
engineers flag as potentially dangerous cases during their research. 
Since the patches we submit address long-standing vulnerabilities, we 
have to maintain them on our side for years, as they rarely get fixed in 
your upstream. If you think these findings are outdated or redundant, 
fine. I'll try to convince our researchers to drop it—letting them know 
that the upstream developers consider this work completely unnecessary.

P.S. We are backporting these from Debian 10 (even though the current 
release is Debian 13).
P.P.S. I used AI for correcting translate (I not good knowled english)

nice day

16.09.2026 15:48, Markus Armbruster пишет:
> First, please read docs/devel/code-provenance.rst section "Use of
> AI-generated content".
>
> I'm not looking at the patches, only the commit message.
>
> Николай Зорин <zorin@swemel.ru> writes:
>
>> SWSA#b0709cee-9722-4603-a30c-dc857dc87961
>>
>> 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`.
> This is nonsense.  error_vprepend() and error_prepend() modify an error.
> They must not be called when there is none.  The check isn't missing.
> Adding it makes the functions "work" papers over bugs.
>
>> 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.
>>
>> 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`, calling `error_reportf_err()` with an unallocated/NULL
>>     `local_error` is incorrect and unsafe. Replace it with a straightforward
>>     `error_report()` call.
> Both uses of error_reportf_err() are indeed wrong.
>
>> - Fix a cosmetic double-space typo in "Error loading  back-end state".
> Worth cleaning up.
>
>> Signed-off-by: Nikolay N Zorin <zorin@swemel.ru>

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

[Michael S. Tsirkin]

On Wed, Sep 16, 2026 at 04:48:19PM +0300, Николай Зорин wrote:
> Hello!
> 
> This is not AI-generated content. We have a dedicated team of engineers who
> monitor and analyze security events (ranging from ASan/TSan sanitizers to
> PVS-Studio) and perform statistical analysis (from cppcheck to SVACE and
> white bear). This is a strict requirement from our Quality Assurance
> department, which is mandatory for compliance with our government and
> commercial contracts. Unfortunately, I cannot share our internal security
> tools (especially since they are tailored to our environment and won't work
> for you). However, I can share what our engineers flag as potentially
> dangerous cases during their research. Since the patches we submit address
> long-standing vulnerabilities, we have to maintain them on our side for
> years, as they rarely get fixed in your upstream. If you think these
> findings are outdated or redundant, fine.

I don't know. The patches aren't documented to the level where I can
pass judgement. what there is, does not match code.

"We have an internal tool that flags this code as a bug, I'm not telling
you what it is and I have no idea if the patch is right, it makes my
tool shut up and my government happy, figure the rest out yourself" is
not how any of this works.

If you think your changes are important, work with your dedicated team
to figure out how to be prepared to defend them in public.

Good format to follow:

At the moment .... as a result ... but ... so ... and we can not ...
Fix this by ... as a result ...



> I'll try to convince our
> researchers to drop it—letting them know that the upstream developers
> consider this work completely unnecessary.

I merely consider it incomplete and poorly documented.

> P.S. We are backporting these from Debian 10 (even though the current
> release is Debian 13).

Shrug. This is not the tree against which qemu-devel patches are
supposed to be against.

> P.P.S. I used AI for correcting translate (I not good knowled english)
> 
> nice day
> 
> 16.09.2026 15:48, Markus Armbruster пишет:
> > First, please read docs/devel/code-provenance.rst section "Use of
> > AI-generated content".
> > 
> > I'm not looking at the patches, only the commit message.
> > 
> > Николай Зорин <zorin@swemel.ru> writes:
> > 
> > > SWSA#b0709cee-9722-4603-a30c-dc857dc87961
> > > 
> > > 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`.
> > This is nonsense.  error_vprepend() and error_prepend() modify an error.
> > They must not be called when there is none.  The check isn't missing.
> > Adding it makes the functions "work" papers over bugs.
> > 
> > > 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.
> > > 
> > > 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`, calling `error_reportf_err()` with an unallocated/NULL
> > >     `local_error` is incorrect and unsafe. Replace it with a straightforward
> > >     `error_report()` call.
> > Both uses of error_reportf_err() are indeed wrong.
> > 
> > > - Fix a cosmetic double-space typo in "Error loading  back-end state".
> > Worth cleaning up.
> > 
> > > Signed-off-by: Nikolay N Zorin <zorin@swemel.ru>

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

[Markus Armbruster]

Николай Зорин <zorin@swemel.ru> writes:

> Hello!
>
> This is not AI-generated content. We have a dedicated team of engineers who monitor and analyze security events (ranging from ASan/TSan sanitizers to PVS-Studio) and perform statistical analysis (from cppcheck to SVACE and white bear). This is a strict requirement from our Quality Assurance department, which is mandatory for compliance with our government and commercial contracts. Unfortunately, I cannot share our internal security tools (especially since they are tailored to our environment and won't work for you). However, I can share what our engineers flag as potentially dangerous cases during their research. Since the patches we submit address long-standing vulnerabilities, we have to maintain them on our side for years, as they rarely get fixed in your upstream. If you think these findings are outdated or redundant, fine. I'll try to convince our researchers to drop it—letting them know that the upstream developers consider this work completely unnecessary.
>
> P.S. We are backporting these from Debian 10 (even though the current release is Debian 13).
> P.P.S. I used AI for correcting translate (I not good knowled english)
>
> nice day

When I smell AI, I ask.  This isn't me judging the patch submitter.
It's simply my duty as a maintainer to make a reasonable effort to
ensure patch submitters know what they're doing.

According to the commit message, your patch is about three separate
issues:

1. error_vprepend() crashes when misused

   Yes it does, and that's deliberate.  If you can see a away to get it
   to crash, there's a bug in need of fixing further up the call chain.
   We'd appreciate a patch then.  Alternatively, a reproducer and/or
   stack backtrace, or even static analysis.

2. vhost_user_net_save_state() and vhost_user_net_load_state() misuse
   error_reportf_err()

   They do.  Have you searched for additional instances of this bug?
   Question, not a demand!

3. A cosmetic issue in an error message nearby.

   Yup, there's a double space.  You could go the extra mile and search
   for additional instances.  Again, not a demand.

We prefer one patch per issue, although nearby trivial cleanups can
sometimes be thrown in.  Doing 3 together with 2 is fine.