1. Patchew
  2. QEMU
  3. [PATCH 0/3] migration: A few fixes on coverity reports
  4. View patch

[PATCH 1/3] migration: Fix error leak in postcopy_ram_listen_thread()

Peter Xu posted 3 patches 3 weeks, 3 days ago
Maintainers: Steve Sistare <steven.sistare@oracle.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
There is a newer version of this series
[PATCH 1/3] migration: Fix error leak in postcopy_ram_listen_thread()
Posted by Peter Xu 3 weeks, 3 days ago 
As reported and analyzed by Peter:

https://lore.kernel.org/r/CAFEAcA9otBWtR7rPQ0Y9aBm+7ZWJzd4VWpXrAmGr8XspPn+zpw@mail.gmail.com

Fix it by freeing the error.  When at it, always reset the local_err
pointer in both paths.

Cc: Arun Menon <armenon@redhat.com>
Resolves: Coverity CID 1641390
Fixes: 94272d9b45 ("migration: Capture error in postcopy_ram_listen_thread()")
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index aafa40d779..635fa2f918 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2136,17 +2136,18 @@ static void *postcopy_ram_listen_thread(void *opaque)
         if (postcopy_state_get() == POSTCOPY_INCOMING_RUNNING &&
             !migrate_postcopy_ram() && migrate_dirty_bitmaps())
         {
-            error_report("%s: loadvm failed during postcopy: %d. All states "
+            error_report("%s: loadvm failed during postcopy: %s. All states "
                          "are migrated except dirty bitmaps. Some dirty "
                          "bitmaps may be lost, and present migrated dirty "
                          "bitmaps are correctly migrated and valid.",
-                         __func__, load_res);
+                         __func__, error_get_pretty(local_err));
+            g_clear_pointer(&local_err, error_free);
             load_res = 0; /* prevent further exit() */
         } else {
             error_prepend(&local_err,
                           "loadvm failed during postcopy: %d: ", load_res);
             migrate_set_error(migr, local_err);
-            error_report_err(local_err);
+            g_clear_pointer(&local_err, error_report_err);
             migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
                                            MIGRATION_STATUS_FAILED);
         }
-- 
2.50.1
Re: [PATCH 1/3] migration: Fix error leak in postcopy_ram_listen_thread()
Posted by Fabiano Rosas 3 weeks, 2 days ago 
Peter Xu <peterx@redhat.com> writes:

> As reported and analyzed by Peter:
>
> https://lore.kernel.org/r/CAFEAcA9otBWtR7rPQ0Y9aBm+7ZWJzd4VWpXrAmGr8XspPn+zpw@mail.gmail.com
>
> Fix it by freeing the error.  When at it, always reset the local_err
> pointer in both paths.
>
> Cc: Arun Menon <armenon@redhat.com>
> Resolves: Coverity CID 1641390
> Fixes: 94272d9b45 ("migration: Capture error in postcopy_ram_listen_thread()")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index aafa40d779..635fa2f918 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2136,17 +2136,18 @@ static void *postcopy_ram_listen_thread(void *opaque)
>          if (postcopy_state_get() == POSTCOPY_INCOMING_RUNNING &&
>              !migrate_postcopy_ram() && migrate_dirty_bitmaps())
>          {
> -            error_report("%s: loadvm failed during postcopy: %d. All states "
> +            error_report("%s: loadvm failed during postcopy: %s. All states "

Do we want to keep the %d for consistency with the way we report the
error below?

loadvm failed during postcopy: %d: %s

>                           "are migrated except dirty bitmaps. Some dirty "
>                           "bitmaps may be lost, and present migrated dirty "
>                           "bitmaps are correctly migrated and valid.",
> -                         __func__, load_res);
> +                         __func__, error_get_pretty(local_err));
> +            g_clear_pointer(&local_err, error_free);
>              load_res = 0; /* prevent further exit() */
>          } else {
>              error_prepend(&local_err,
>                            "loadvm failed during postcopy: %d: ", load_res);
>              migrate_set_error(migr, local_err);
> -            error_report_err(local_err);
> +            g_clear_pointer(&local_err, error_report_err);
>              migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
>                                             MIGRATION_STATUS_FAILED);
>          }
Re: [PATCH 1/3] migration: Fix error leak in postcopy_ram_listen_thread()
Posted by Peter Xu 3 weeks, 2 days ago 
On Tue, Oct 21, 2025 at 05:45:12PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > As reported and analyzed by Peter:
> >
> > https://lore.kernel.org/r/CAFEAcA9otBWtR7rPQ0Y9aBm+7ZWJzd4VWpXrAmGr8XspPn+zpw@mail.gmail.com
> >
> > Fix it by freeing the error.  When at it, always reset the local_err
> > pointer in both paths.
> >
> > Cc: Arun Menon <armenon@redhat.com>
> > Resolves: Coverity CID 1641390
> > Fixes: 94272d9b45 ("migration: Capture error in postcopy_ram_listen_thread()")
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/savevm.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index aafa40d779..635fa2f918 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -2136,17 +2136,18 @@ static void *postcopy_ram_listen_thread(void *opaque)
> >          if (postcopy_state_get() == POSTCOPY_INCOMING_RUNNING &&
> >              !migrate_postcopy_ram() && migrate_dirty_bitmaps())
> >          {
> > -            error_report("%s: loadvm failed during postcopy: %d. All states "
> > +            error_report("%s: loadvm failed during postcopy: %s. All states "
> 
> Do we want to keep the %d for consistency with the way we report the
> error below?
> 
> loadvm failed during postcopy: %d: %s

I thought the string is verbose enough in an error report, but sure, I can
do that.

> 
> >                           "are migrated except dirty bitmaps. Some dirty "
> >                           "bitmaps may be lost, and present migrated dirty "
> >                           "bitmaps are correctly migrated and valid.",
> > -                         __func__, load_res);
> > +                         __func__, error_get_pretty(local_err));
> > +            g_clear_pointer(&local_err, error_free);
> >              load_res = 0; /* prevent further exit() */
> >          } else {
> >              error_prepend(&local_err,
> >                            "loadvm failed during postcopy: %d: ", load_res);
> >              migrate_set_error(migr, local_err);
> > -            error_report_err(local_err);
> > +            g_clear_pointer(&local_err, error_report_err);
> >              migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
> >                                             MIGRATION_STATUS_FAILED);
> >          }
> 

-- 
Peter Xu

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.