block/vmdk: prevent double-free in extent memory management

[PATCH] block/vmdk: prevent double-free in extent memory management

gerben@altlinux.org posted 1 patch 11 months, 1 week ago

Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250304090415.39393-1-gerben@altlinux.org

Maintainers: Fam Zheng <fam@euphon.net>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>

block/vmdk.c | 1 +
1 file changed, 1 insertion(+)

Expand all Fold all

[PATCH] block/vmdk: prevent double-free in extent memory management

Posted by gerben@altlinux.org 11 months, 1 week ago

From: Denis Rastyogin <gerben@altlinux.org>

This error was discovered by fuzzing qemu-img.

A double-free issue in the VMDK driver occurs when handling snapshots.
The memory allocated for extent structures is freed twice: first in
vmdk_close (block/vmdk.c) and then in vmdk_add_extent (block/vmdk.c).

The fix ensures the s->extents pointer is set to NULL after freeing,
preventing double-free.

Closes: https://gitlab.com/qemu-project/qemu/-/issues/2853
Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
---
 block/vmdk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2adec49912..d6baa54602 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -285,6 +285,7 @@ static void vmdk_free_extents(BlockDriverState *bs)
     bdrv_graph_wrunlock();
 
     g_free(s->extents);
+    s->extents = NULL;
 }
 
 static void vmdk_free_last_extent(BlockDriverState *bs)
-- 
2.42.2

Re: [PATCH] block/vmdk: prevent double-free in extent memory management

Posted by Kevin Wolf 11 months ago

Am 04.03.2025 um 10:04 hat gerben@altlinux.org geschrieben:
> From: Denis Rastyogin <gerben@altlinux.org>
> 
> This error was discovered by fuzzing qemu-img.
> 
> A double-free issue in the VMDK driver occurs when handling snapshots.
> The memory allocated for extent structures is freed twice: first in
> vmdk_close (block/vmdk.c) and then in vmdk_add_extent (block/vmdk.c).
> 
> The fix ensures the s->extents pointer is set to NULL after freeing,
> preventing double-free.
> 
> Closes: https://gitlab.com/qemu-project/qemu/-/issues/2853
> Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
> ---
>  block/vmdk.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 2adec49912..d6baa54602 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -285,6 +285,7 @@ static void vmdk_free_extents(BlockDriverState *bs)
>      bdrv_graph_wrunlock();
>  
>      g_free(s->extents);
> +    s->extents = NULL;
>  }

This is not the right fix. It only papers over the real bug, which is
that bdrv_snapshot_goto() calls .bdrv_open() for a BlockDriverState
whose bs->opaque hasn't been zeroed. Open functions of block drivers
generally rely on having a zeroed state.

Kevin