[PATCH] replay: fix use of uninitialized pointer on error

marcandre.lureau@redhat.com posted 1 patch 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260719113216.1177594-1-marcandre.lureau@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>
replay/replay-debugging.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
[PATCH] replay: fix use of uninitialized pointer on error
Posted by marcandre.lureau@redhat.com 6 days ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

When bdrv_snapshot_list() returns a negative error code, sn_tab is
uninitialized. The loop does not execute (since i=0 < negative is
false), but the code falls through to g_free(sn_tab) which frees
an uninitialized pointer.

Fixes: f6baed3d1485 ("replay: implement replay-seek command")
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
 replay/replay-debugging.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 11053640021..7b904a14165 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -139,9 +139,8 @@ static char *replay_find_nearest_snapshot(int64_t icount,
                                           int64_t *snapshot_icount)
 {
     BlockDriverState *bs;
-    QEMUSnapshotInfo *sn_tab;
+    g_autofree QEMUSnapshotInfo *sn_tab = NULL;
     QEMUSnapshotInfo *nearest = NULL;
-    char *ret = NULL;
     int rv;
     int nb_sns, i;
 
@@ -149,15 +148,19 @@ static char *replay_find_nearest_snapshot(int64_t icount,
 
     bs = bdrv_all_find_vmstate_bs(NULL, false, NULL, NULL);
     if (!bs) {
-        goto fail;
+        return NULL;
     }
 
     nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+    if (nb_sns < 0) {
+        return NULL;
+    }
 
     for (i = 0; i < nb_sns; i++) {
         rv = bdrv_all_has_snapshot(sn_tab[i].name, false, NULL, NULL);
-        if (rv < 0)
-            goto fail;
+        if (rv < 0) {
+            return NULL;
+        }
         if (rv == 1) {
             if (sn_tab[i].icount != -1ULL
                 && sn_tab[i].icount <= icount
@@ -166,14 +169,12 @@ static char *replay_find_nearest_snapshot(int64_t icount,
             }
         }
     }
-    if (nearest) {
-        ret = g_strdup(nearest->name);
-        *snapshot_icount = nearest->icount;
+    if (!nearest) {
+        return NULL;
     }
-    g_free(sn_tab);
 
-fail:
-    return ret;
+    *snapshot_icount = nearest->icount;
+    return g_strdup(nearest->name);
 }
 
 static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)
-- 
2.55.0


Re: [PATCH] replay: fix use of uninitialized pointer on error
Posted by Michael Tokarev 3 days, 2 hours ago
On 7/19/26 14:32, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> When bdrv_snapshot_list() returns a negative error code, sn_tab is
> uninitialized. The loop does not execute (since i=0 < negative is
> false), but the code falls through to g_free(sn_tab) which frees
> an uninitialized pointer.
> 
> Fixes: f6baed3d1485 ("replay: implement replay-seek command")
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   replay/replay-debugging.c | 23 ++++++++++++-----------
>   1 file changed, 12 insertions(+), 11 deletions(-)

I'm picking this up for the current stable qemu series.
Please let me know if I should not.

Thanks,

/mjt

Re: [PATCH] replay: fix use of uninitialized pointer on error
Posted by Philippe Mathieu-Daudé 4 days, 1 hour ago
On 19/7/26 13:32, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> When bdrv_snapshot_list() returns a negative error code, sn_tab is
> uninitialized. The loop does not execute (since i=0 < negative is
> false), but the code falls through to g_free(sn_tab) which frees
> an uninitialized pointer.
> 
> Fixes: f6baed3d1485 ("replay: implement replay-seek command")
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   replay/replay-debugging.c | 23 ++++++++++++-----------
>   1 file changed, 12 insertions(+), 11 deletions(-)

Patch queued, thanks.

Re: [PATCH] replay: fix use of uninitialized pointer on error
Posted by Philippe Mathieu-Daudé 5 days, 4 hours ago
On 19/7/26 13:32, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> When bdrv_snapshot_list() returns a negative error code, sn_tab is
> uninitialized. The loop does not execute (since i=0 < negative is
> false), but the code falls through to g_free(sn_tab) which frees
> an uninitialized pointer.
> 
> Fixes: f6baed3d1485 ("replay: implement replay-seek command")
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   replay/replay-debugging.c | 23 ++++++++++++-----------
>   1 file changed, 12 insertions(+), 11 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH] replay: fix use of uninitialized pointer on error
Posted by Philippe Mathieu-Daudé 5 days, 4 hours ago
On 19/7/26 13:32, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> When bdrv_snapshot_list() returns a negative error code, sn_tab is
> uninitialized. The loop does not execute (since i=0 < negative is
> false), but the code falls through to g_free(sn_tab) which frees
> an uninitialized pointer.
> 
> Fixes: f6baed3d1485 ("replay: implement replay-seek command")
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   replay/replay-debugging.c | 23 ++++++++++++-----------
>   1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
> index 11053640021..7b904a14165 100644
> --- a/replay/replay-debugging.c
> +++ b/replay/replay-debugging.c
> @@ -139,9 +139,8 @@ static char *replay_find_nearest_snapshot(int64_t icount,
>                                             int64_t *snapshot_icount)
>   {
>       BlockDriverState *bs;
> -    QEMUSnapshotInfo *sn_tab;
> +    g_autofree QEMUSnapshotInfo *sn_tab = NULL;
>       QEMUSnapshotInfo *nearest = NULL;
> -    char *ret = NULL;
>       int rv;
>       int nb_sns, i;
>   
> @@ -149,15 +148,19 @@ static char *replay_find_nearest_snapshot(int64_t icount,
>   
>       bs = bdrv_all_find_vmstate_bs(NULL, false, NULL, NULL);
>       if (!bs) {
> -        goto fail;
> +        return NULL;
>       }
>   
>       nb_sns = bdrv_snapshot_list(bs, &sn_tab);
> +    if (nb_sns < 0) {
> +        return NULL;
> +    }
>   
>       for (i = 0; i < nb_sns; i++) {
>           rv = bdrv_all_has_snapshot(sn_tab[i].name, false, NULL, NULL);
> -        if (rv < 0)
> -            goto fail;
> +        if (rv < 0) {
> +            return NULL;
> +        }
>           if (rv == 1) {
>               if (sn_tab[i].icount != -1ULL
>                   && sn_tab[i].icount <= icount
> @@ -166,14 +169,12 @@ static char *replay_find_nearest_snapshot(int64_t icount,
>               }
>           }
>       }
> -    if (nearest) {
> -        ret = g_strdup(nearest->name);
> -        *snapshot_icount = nearest->icount;
> +    if (!nearest) {
> +        return NULL;
>       }
> -    g_free(sn_tab);
>   
> -fail:
> -    return ret;
> +    *snapshot_icount = nearest->icount;
> +    return g_strdup(nearest->name);
>   }
>   
>   static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)