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

Peter Xu posted 4 patches 3 weeks, 2 days ago
Maintainers: Steve Sistare <steven.sistare@oracle.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
[PATCH v2 1/4] migration: Fix error leak in postcopy_ram_listen_thread()
Posted by Peter Xu 3 weeks, 2 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..232cae090b 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: %d: %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__, load_res, 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 v2 1/4] 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..232cae090b 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: %d: %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__, load_res, 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);
>          }

Reviewed-by: Fabiano Rosas <farosas@suse.de>