[PATCH 3/6] dump/win_dump: fix use after free of err

Vladimir Sementsov-Ogievskiy posted 6 patches 5 years, 10 months ago
Maintainers: Max Reitz <mreitz@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, John Snow <jsnow@redhat.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, Kevin Wolf <kwolf@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Juan Quintela <quintela@redhat.com>, Hailiang Zhang <zhang.zhanghailiang@huawei.com>
[PATCH 3/6] dump/win_dump: fix use after free of err
Posted by Vladimir Sementsov-Ogievskiy 5 years, 10 months ago
It's possible that we'll try to set err twice (or more). It's bad, it
will crash.

Instead, use warn_report().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 dump/win_dump.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/dump/win_dump.c b/dump/win_dump.c
index eda2a48974..652c7bad99 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -304,13 +304,11 @@ static void restore_context(WinDumpHeader64 *h,
                             struct saved_context *saved_ctx)
 {
     int i;
-    Error *err = NULL;
 
     for (i = 0; i < h->NumberProcessors; i++) {
         if (cpu_memory_rw_debug(first_cpu, saved_ctx[i].addr,
                 (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext), 1)) {
-            error_setg(&err, "win-dump: failed to restore CPU #%d context", i);
-            warn_report_err(err);
+            warn_report("win-dump: failed to restore CPU #%d context", i);
         }
     }
 }
-- 
2.21.0


Re: [PATCH 3/6] dump/win_dump: fix use after free of err
Posted by Markus Armbruster 5 years, 10 months ago
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> It's possible that we'll try to set err twice (or more). It's bad, it
> will crash.

True.

> Instead, use warn_report().

Improvement even without the potential crash enabled by the loop.

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  dump/win_dump.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index eda2a48974..652c7bad99 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -304,13 +304,11 @@ static void restore_context(WinDumpHeader64 *h,
>                              struct saved_context *saved_ctx)
>  {
>      int i;
> -    Error *err = NULL;
>  
>      for (i = 0; i < h->NumberProcessors; i++) {
>          if (cpu_memory_rw_debug(first_cpu, saved_ctx[i].addr,
>                  (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext), 1)) {
> -            error_setg(&err, "win-dump: failed to restore CPU #%d context", i);
> -            warn_report_err(err);
> +            warn_report("win-dump: failed to restore CPU #%d context", i);
>          }
>      }
>  }

Reviewed-by: Markus Armbruster <armbru@redhat.com>