1. Patchew
  2. QEMU
  3. View series

[PATCH] block: Zero block driver state before reopening

Kevin Wolf posted 1 patch 3 weeks, 2 days ago 
block/snapshot.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] block: Zero block driver state before reopening
Posted by Kevin Wolf 3 weeks, 2 days ago 
Block drivers assume in their .bdrv_open() implementation that their
state in bs->opaque has been zeroed; it is initially allocated with
g_malloc0() in bdrv_open_driver().

bdrv_snapshot_goto() needs to make sure that it is zeroed again before
calling drv->bdrv_open() to avoid that block drivers use stale values.

One symptom of this bug is VMDK running into a double free when the user
tries to apply an internal snapshot like 'qemu-img snapshot -a test
test.vmdk'. This should be a graceful error because VMDK doesn't support
internal snapshots.

==25507== Invalid free() / delete / delete[] / realloc()
==25507==    at 0x484B347: realloc (vg_replace_malloc.c:1801)
==25507==    by 0x54B592A: g_realloc (gmem.c:171)
==25507==    by 0x1B221D: vmdk_add_extent (../block/vmdk.c:570)
==25507==    by 0x1B1084: vmdk_open_sparse (../block/vmdk.c:1059)
==25507==    by 0x1AF3D8: vmdk_open (../block/vmdk.c:1371)
==25507==    by 0x1A2AE0: bdrv_snapshot_goto (../block/snapshot.c:299)
==25507==    by 0x205C77: img_snapshot (../qemu-img.c:3500)
==25507==    by 0x58FA087: (below main) (libc_start_call_main.h:58)
==25507==  Address 0x832f3e0 is 0 bytes inside a block of size 272 free'd
==25507==    at 0x4846B83: free (vg_replace_malloc.c:989)
==25507==    by 0x54AEAC4: g_free (gmem.c:208)
==25507==    by 0x1AF629: vmdk_close (../block/vmdk.c:2889)
==25507==    by 0x1A2A9C: bdrv_snapshot_goto (../block/snapshot.c:290)
==25507==    by 0x205C77: img_snapshot (../qemu-img.c:3500)
==25507==    by 0x58FA087: (below main) (libc_start_call_main.h:58)

This error was discovered by fuzzing qemu-img.

Cc: qemu-stable@nongnu.org
Reported-by: Denis Rastyogin <gerben@altlinux.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/snapshot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/snapshot.c b/block/snapshot.c
index 9c44780e96..22567f1fb9 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -296,6 +296,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
         bdrv_graph_wrunlock();
 
         ret = bdrv_snapshot_goto(fallback_bs, snapshot_id, errp);
+        memset(bs->opaque, 0, drv->instance_size);
         open_ret = drv->bdrv_open(bs, options, bs->open_flags, &local_err);
         qobject_unref(options);
         if (open_ret < 0) {
-- 
2.48.1
Re: [PATCH] block: Zero block driver state before reopening
Posted by Eric Blake 1 week, 6 days ago 
On Mon, Mar 10, 2025 at 11:48:58AM +0100, Kevin Wolf wrote:
> Block drivers assume in their .bdrv_open() implementation that their
> state in bs->opaque has been zeroed; it is initially allocated with
> g_malloc0() in bdrv_open_driver().
> 
> bdrv_snapshot_goto() needs to make sure that it is zeroed again before
> calling drv->bdrv_open() to avoid that block drivers use stale values.
> 
> One symptom of this bug is VMDK running into a double free when the user
> tries to apply an internal snapshot like 'qemu-img snapshot -a test
> test.vmdk'. This should be a graceful error because VMDK doesn't support
> internal snapshots.
> 
> ==25507== Invalid free() / delete / delete[] / realloc()
> ==25507==    at 0x484B347: realloc (vg_replace_malloc.c:1801)
> ==25507==    by 0x54B592A: g_realloc (gmem.c:171)
> ==25507==    by 0x1B221D: vmdk_add_extent (../block/vmdk.c:570)
> ==25507==    by 0x1B1084: vmdk_open_sparse (../block/vmdk.c:1059)
> ==25507==    by 0x1AF3D8: vmdk_open (../block/vmdk.c:1371)
> ==25507==    by 0x1A2AE0: bdrv_snapshot_goto (../block/snapshot.c:299)
> ==25507==    by 0x205C77: img_snapshot (../qemu-img.c:3500)
> ==25507==    by 0x58FA087: (below main) (libc_start_call_main.h:58)
> ==25507==  Address 0x832f3e0 is 0 bytes inside a block of size 272 free'd
> ==25507==    at 0x4846B83: free (vg_replace_malloc.c:989)
> ==25507==    by 0x54AEAC4: g_free (gmem.c:208)
> ==25507==    by 0x1AF629: vmdk_close (../block/vmdk.c:2889)
> ==25507==    by 0x1A2A9C: bdrv_snapshot_goto (../block/snapshot.c:290)
> ==25507==    by 0x205C77: img_snapshot (../qemu-img.c:3500)
> ==25507==    by 0x58FA087: (below main) (libc_start_call_main.h:58)
> 
> This error was discovered by fuzzing qemu-img.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Denis Rastyogin <gerben@altlinux.org>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/snapshot.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
Re: [PATCH] block: Zero block driver state before reopening
Posted by Kevin Wolf 3 weeks, 2 days ago 
Am 10.03.2025 um 11:48 hat Kevin Wolf geschrieben:
> Block drivers assume in their .bdrv_open() implementation that their
> state in bs->opaque has been zeroed; it is initially allocated with
> g_malloc0() in bdrv_open_driver().
> 
> bdrv_snapshot_goto() needs to make sure that it is zeroed again before
> calling drv->bdrv_open() to avoid that block drivers use stale values.
> 
> One symptom of this bug is VMDK running into a double free when the user
> tries to apply an internal snapshot like 'qemu-img snapshot -a test
> test.vmdk'. This should be a graceful error because VMDK doesn't support
> internal snapshots.
> 
> ==25507== Invalid free() / delete / delete[] / realloc()
> ==25507==    at 0x484B347: realloc (vg_replace_malloc.c:1801)
> ==25507==    by 0x54B592A: g_realloc (gmem.c:171)
> ==25507==    by 0x1B221D: vmdk_add_extent (../block/vmdk.c:570)
> ==25507==    by 0x1B1084: vmdk_open_sparse (../block/vmdk.c:1059)
> ==25507==    by 0x1AF3D8: vmdk_open (../block/vmdk.c:1371)
> ==25507==    by 0x1A2AE0: bdrv_snapshot_goto (../block/snapshot.c:299)
> ==25507==    by 0x205C77: img_snapshot (../qemu-img.c:3500)
> ==25507==    by 0x58FA087: (below main) (libc_start_call_main.h:58)
> ==25507==  Address 0x832f3e0 is 0 bytes inside a block of size 272 free'd
> ==25507==    at 0x4846B83: free (vg_replace_malloc.c:989)
> ==25507==    by 0x54AEAC4: g_free (gmem.c:208)
> ==25507==    by 0x1AF629: vmdk_close (../block/vmdk.c:2889)
> ==25507==    by 0x1A2A9C: bdrv_snapshot_goto (../block/snapshot.c:290)
> ==25507==    by 0x205C77: img_snapshot (../qemu-img.c:3500)
> ==25507==    by 0x58FA087: (below main) (libc_start_call_main.h:58)
> 
> This error was discovered by fuzzing qemu-img.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Denis Rastyogin <gerben@altlinux.org>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Forgot these two lines, which I'll add while applying:

Closes: https://gitlab.com/qemu-project/qemu/-/issues/2853
Closes: https://gitlab.com/qemu-project/qemu/-/issues/2851

Kevin

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.