| Message ID | 20250310104858.28221-1-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: Zero block driver state before reopening | expand |
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
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) {
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(+)