[PATCH 0/6] net: Fix various Coverity issues in vhost-user and passt

[PATCH 0/6] net: Fix various Coverity issues in vhost-user and passt

[Laurent Vivier]

This patch series addresses several static analysis warnings reported by
Coverity (CIDs 1612365, 1612368, 1612369, 1612371, 1612372, 1612375)
in the vhost-user and passt networking code.

The changes are mostly minor cleanups and fixes:

Three patches remove an unused "err" variable and its associated
unreachable error handling block.

One patch removes another piece of dead code in an error path by
refactoring away a goto.

The remaining two patches initialize a pointer to prevent a potential
illegal memory access and add a check for the return value of
g_remove() to log potential errors.

Laurent Vivier (6):
  net/passt: Remove unused "err" from passt_vhost_user_event() (CID
    1612375)
  net/vhost-user: Remove unused "err" from net_vhost_user_event() (CID
    1612372)
  net/passt: Remove dead code in passt_vhost_user_start error path (CID
    1612371)
  net/passt: Check return value of g_remove() in net_passt_cleanup()
    (CID 1612369)
  net/passt: Initialize "error" variable in net_passt_send() (CID
    1612368)
  net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365)

 net/passt.c      | 22 +++++++---------------
 net/vhost-user.c |  9 ---------
 2 files changed, 7 insertions(+), 24 deletions(-)

-- 
2.49.0

[PATCH 1/6] net/passt: Remove unused "err" from passt_vhost_user_event() (CID 1612375)

[Laurent Vivier]

The "err" variable was declared but never used within the
passt_vhost_user_event() function. This resulted in a dead code
warning (CID 1612375) from Coverity.

Remove the unused variable and the associated error block
to resolve the issue.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 net/passt.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/passt.c b/net/passt.c
index 6f616ba3c258..9cd5b3e6f2af 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -397,7 +397,6 @@ err:
 static void passt_vhost_user_event(void *opaque, QEMUChrEvent event)
 {
     NetPasstState *s = opaque;
-    Error *err = NULL;
 
     switch (event) {
     case CHR_EVENT_OPENED:
@@ -428,10 +427,6 @@ static void passt_vhost_user_event(void *opaque, QEMUChrEvent event)
         /* Ignore */
         break;
     }
-
-    if (err) {
-        error_report_err(err);
-    }
 }
 
 static int net_passt_vhost_user_init(NetPasstState *s, Error **errp)
-- 
2.49.0

[PATCH 2/6] net/vhost-user: Remove unused "err" from net_vhost_user_event() (CID 1612372)

[Laurent Vivier]

The "err" variable was declared but never used within the
net_vhost_user_event() function. This resulted in a dead code
warning (CID 1612372) from Coverity.

Remove the unused variable and the associated error block
to resolve the issue.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 net/vhost-user.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 1c3b8b36f351..cec83e925ff0 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -329,7 +329,6 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
     NetClientState *ncs[MAX_QUEUE_NUM];
     NetVhostUserState *s;
     Chardev *chr;
-    Error *err = NULL;
     int queues;
 
     queues = qemu_find_net_clients_except(name, ncs,
@@ -375,10 +374,6 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
         /* Ignore */
         break;
     }
-
-    if (err) {
-        error_report_err(err);
-    }
 }
 
 static int net_vhost_user_init(NetClientState *peer, const char *device,
-- 
2.49.0

[PATCH 3/6] net/passt: Remove dead code in passt_vhost_user_start error path (CID 1612371)

[Laurent Vivier]

In passt_vhost_user_start(), if vhost_net_init() fails, the "net"
variable is NULL and execution jumps to the "err:" label.

The cleanup code within this label is conditioned on "if (net)",
which can never be true in this error case. This makes the cleanup
block dead code, as reported by Coverity (CID 1612371).

Refactor the error handling to occur inline, removing the goto and
the unreachable cleanup block.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 net/passt.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/passt.c b/net/passt.c
index 9cd5b3e6f2af..ef59d0682be1 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -375,7 +375,8 @@ static int passt_vhost_user_start(NetPasstState *s, VhostUserState *be)
     net = vhost_net_init(&options);
     if (!net) {
         error_report("failed to init passt vhost_net");
-        goto err;
+        passt_vhost_user_stop(s);
+        return -1;
     }
 
     if (s->vhost_net) {
@@ -385,13 +386,6 @@ static int passt_vhost_user_start(NetPasstState *s, VhostUserState *be)
     s->vhost_net = net;
 
     return 0;
-err:
-    if (net) {
-        vhost_net_cleanup(net);
-        g_free(net);
-    }
-    passt_vhost_user_stop(s);
-    return -1;
 }
 
 static void passt_vhost_user_event(void *opaque, QEMUChrEvent event)
-- 
2.49.0

[PATCH 4/6] net/passt: Check return value of g_remove() in net_passt_cleanup() (CID 1612369)

[Laurent Vivier]

If g_remove() fails, use warn_report() to log an error.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 net/passt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/passt.c b/net/passt.c
index ef59d0682be1..43c336e5968c 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -103,7 +103,10 @@ static void net_passt_cleanup(NetClientState *nc)
 #endif
 
     kill(s->pid, SIGTERM);
-    g_remove(s->pidfile);
+    if (g_remove(s->pidfile) != 0) {
+        warn_report("Failed to remove passt pidfile %s: %s",
+                    s->pidfile, strerror(errno));
+    }
     g_free(s->pidfile);
     g_ptr_array_free(s->args, TRUE);
 }
-- 
2.49.0

[PATCH 5/6] net/passt: Initialize "error" variable in net_passt_send() (CID 1612368)

[Laurent Vivier]

This was flagged by Coverity as a memory illegal access.

Initialize the pointer to NULL at declaration.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 net/passt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/passt.c b/net/passt.c
index 43c336e5968c..32ecffb763b4 100644
--- a/net/passt.c
+++ b/net/passt.c
@@ -124,7 +124,7 @@ static gboolean net_passt_send(QIOChannel *ioc, GIOCondition condition,
 {
     if (net_stream_data_send(ioc, condition, data) == G_SOURCE_REMOVE) {
         NetPasstState *s = DO_UPCAST(NetPasstState, data, data);
-        Error *error;
+        Error *error = NULL;
 
         /* we need to restart passt */
         kill(s->pid, SIGTERM);
-- 
2.49.0

[PATCH 6/6] net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365)

[Laurent Vivier]

The "err" variable was declared but never used within the
chr_closed_bh() function. This resulted in a dead code
warning (CID 1612365) from Coverity.

Remove the unused variable and the associated error block
to resolve the issue.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 net/vhost-user.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index cec83e925ff0..8b96157145a7 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -298,7 +298,6 @@ static void chr_closed_bh(void *opaque)
     const char *name = opaque;
     NetClientState *ncs[MAX_QUEUE_NUM];
     NetVhostUserState *s;
-    Error *err = NULL;
     int queues, i;
 
     queues = qemu_find_net_clients_except(name, ncs,
@@ -317,9 +316,6 @@ static void chr_closed_bh(void *opaque)
     qemu_chr_fe_set_handlers(&s->chr, NULL, NULL, net_vhost_user_event,
                              NULL, opaque, NULL, true);
 
-    if (err) {
-        error_report_err(err);
-    }
     qapi_event_send_netdev_vhost_user_disconnected(name);
 }
 
-- 
2.49.0

Re: [PATCH 1/6] net/passt: Remove unused "err" from passt_vhost_user_event() (CID 1612375)

[Peter Maydell]

On Thu, 17 Jul 2025 at 18:43, Laurent Vivier <lvivier@redhat.com> wrote:
>
> The "err" variable was declared but never used within the
> passt_vhost_user_event() function. This resulted in a dead code
> warning (CID 1612375) from Coverity.
>
> Remove the unused variable and the associated error block
> to resolve the issue.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 4/6] net/passt: Check return value of g_remove() in net_passt_cleanup() (CID 1612369)

[Peter Maydell]

On Thu, 17 Jul 2025 at 18:44, Laurent Vivier <lvivier@redhat.com> wrote:
>
> If g_remove() fails, use warn_report() to log an error.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  net/passt.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 3/6] net/passt: Remove dead code in passt_vhost_user_start error path (CID 1612371)

[Peter Maydell]

On Thu, 17 Jul 2025 at 18:50, Laurent Vivier <lvivier@redhat.com> wrote:
>
> In passt_vhost_user_start(), if vhost_net_init() fails, the "net"
> variable is NULL and execution jumps to the "err:" label.
>
> The cleanup code within this label is conditioned on "if (net)",
> which can never be true in this error case. This makes the cleanup
> block dead code, as reported by Coverity (CID 1612371).
>
> Refactor the error handling to occur inline, removing the goto and
> the unreachable cleanup block.
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 6/6] net/vhost-user: Remove unused "err" from chr_closed_bh() (CID 1612365)

[Peter Maydell]

On Thu, 17 Jul 2025 at 18:50, Laurent Vivier <lvivier@redhat.com> wrote:
>
> The "err" variable was declared but never used within the
> chr_closed_bh() function. This resulted in a dead code
> warning (CID 1612365) from Coverity.
>
> Remove the unused variable and the associated error block
> to resolve the issue.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  net/vhost-user.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index cec83e925ff0..8b96157145a7 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -298,7 +298,6 @@ static void chr_closed_bh(void *opaque)
>      const char *name = opaque;
>      NetClientState *ncs[MAX_QUEUE_NUM];
>      NetVhostUserState *s;
> -    Error *err = NULL;
>      int queues, i;
>
>      queues = qemu_find_net_clients_except(name, ncs,
> @@ -317,9 +316,6 @@ static void chr_closed_bh(void *opaque)
>      qemu_chr_fe_set_handlers(&s->chr, NULL, NULL, net_vhost_user_event,
>                               NULL, opaque, NULL, true);
>
> -    if (err) {
> -        error_report_err(err);
> -    }
>      qapi_event_send_netdev_vhost_user_disconnected(name);
>  }
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 5/6] net/passt: Initialize "error" variable in net_passt_send() (CID 1612368)

[Peter Maydell]

On Thu, 17 Jul 2025 at 18:51, Laurent Vivier <lvivier@redhat.com> wrote:
>
> This was flagged by Coverity as a memory illegal access.
>
> Initialize the pointer to NULL at declaration.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  net/passt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/passt.c b/net/passt.c
> index 43c336e5968c..32ecffb763b4 100644
> --- a/net/passt.c
> +++ b/net/passt.c
> @@ -124,7 +124,7 @@ static gboolean net_passt_send(QIOChannel *ioc, GIOCondition condition,
>  {
>      if (net_stream_data_send(ioc, condition, data) == G_SOURCE_REMOVE) {
>          NetPasstState *s = DO_UPCAST(NetPasstState, data, data);
> -        Error *error;
> +        Error *error = NULL;
>
>          /* we need to restart passt */
>          kill(s->pid, SIGTERM);
> --
> 2.49.0
>
>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 2/6] net/vhost-user: Remove unused "err" from net_vhost_user_event() (CID 1612372)

[Peter Maydell]

On Thu, 17 Jul 2025 at 18:52, Laurent Vivier <lvivier@redhat.com> wrote:
>
> The "err" variable was declared but never used within the
> net_vhost_user_event() function. This resulted in a dead code
> warning (CID 1612372) from Coverity.
>
> Remove the unused variable and the associated error block
> to resolve the issue.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  net/vhost-user.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 1c3b8b36f351..cec83e925ff0 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -329,7 +329,6 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
>      NetClientState *ncs[MAX_QUEUE_NUM];
>      NetVhostUserState *s;
>      Chardev *chr;
> -    Error *err = NULL;
>      int queues;
>
>      queues = qemu_find_net_clients_except(name, ncs,
> @@ -375,10 +374,6 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
>          /* Ignore */
>          break;
>      }
> -
> -    if (err) {
> -        error_report_err(err);
> -    }
>


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM