Skip to content

Commit

Permalink
block/vdi: Don't take address of fields in packed structs
Browse files Browse the repository at this point in the history
Taking the address of a field in a packed struct is a bad idea, because
it might not be actually aligned enough for that pointer type (and
thus cause a crash on dereference on some host architectures). Newer
versions of clang warn about this.

Instead of passing UUID related functions the address of a possibly
unaligned QemuUUID struct, use local variables and then copy to/from
the struct field as appropriate.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
pm215 authored and kevmw committed Feb 1, 2019
1 parent 0dbaaa7 commit ac928b8
Showing 1 changed file with 25 additions and 13 deletions.
38 changes: 25 additions & 13 deletions block/vdi.c
Expand Up @@ -235,7 +235,8 @@ static void vdi_header_to_le(VdiHeader *header)

static void vdi_header_print(VdiHeader *header)
{
char uuid[37];
char uuidstr[37];
QemuUUID uuid;
logout("text %s", header->text);
logout("signature 0x%08x\n", header->signature);
logout("header size 0x%04x\n", header->header_size);
Expand All @@ -254,14 +255,18 @@ static void vdi_header_print(VdiHeader *header)
logout("block extra 0x%04x\n", header->block_extra);
logout("blocks tot. 0x%04x\n", header->blocks_in_image);
logout("blocks all. 0x%04x\n", header->blocks_allocated);
qemu_uuid_unparse(&header->uuid_image, uuid);
logout("uuid image %s\n", uuid);
qemu_uuid_unparse(&header->uuid_last_snap, uuid);
logout("uuid snap %s\n", uuid);
qemu_uuid_unparse(&header->uuid_link, uuid);
logout("uuid link %s\n", uuid);
qemu_uuid_unparse(&header->uuid_parent, uuid);
logout("uuid parent %s\n", uuid);
uuid = header->uuid_image;
qemu_uuid_unparse(&uuid, uuidstr);
logout("uuid image %s\n", uuidstr);
uuid = header->uuid_last_snap;
qemu_uuid_unparse(&uuid, uuidstr);
logout("uuid snap %s\n", uuidstr);
uuid = header->uuid_link;
qemu_uuid_unparse(&uuid, uuidstr);
logout("uuid link %s\n", uuidstr);
uuid = header->uuid_parent;
qemu_uuid_unparse(&uuid, uuidstr);
logout("uuid parent %s\n", uuidstr);
}

static int coroutine_fn vdi_co_check(BlockDriverState *bs, BdrvCheckResult *res,
Expand Down Expand Up @@ -368,6 +373,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
size_t bmap_size;
int ret;
Error *local_err = NULL;
QemuUUID uuid_link, uuid_parent;

bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
false, errp);
Expand Down Expand Up @@ -395,6 +401,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}

uuid_link = header.uuid_link;
uuid_parent = header.uuid_parent;

if (header.disk_size % SECTOR_SIZE != 0) {
/* 'VBoxManage convertfromraw' can create images with odd disk sizes.
We accept them but round the disk size to the next multiple of
Expand Down Expand Up @@ -444,11 +453,11 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
(uint64_t)header.blocks_in_image * header.block_size);
ret = -ENOTSUP;
goto fail;
} else if (!qemu_uuid_is_null(&header.uuid_link)) {
} else if (!qemu_uuid_is_null(&uuid_link)) {
error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
ret = -ENOTSUP;
goto fail;
} else if (!qemu_uuid_is_null(&header.uuid_parent)) {
} else if (!qemu_uuid_is_null(&uuid_parent)) {
error_setg(errp, "unsupported VDI image (non-NULL parent UUID)");
ret = -ENOTSUP;
goto fail;
Expand Down Expand Up @@ -733,6 +742,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
BlockDriverState *bs_file = NULL;
BlockBackend *blk = NULL;
uint32_t *bmap = NULL;
QemuUUID uuid;

assert(create_options->driver == BLOCKDEV_DRIVER_VDI);
vdi_opts = &create_options->u.vdi;
Expand Down Expand Up @@ -819,8 +829,10 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
if (image_type == VDI_TYPE_STATIC) {
header.blocks_allocated = blocks;
}
qemu_uuid_generate(&header.uuid_image);
qemu_uuid_generate(&header.uuid_last_snap);
qemu_uuid_generate(&uuid);
header.uuid_image = uuid;
qemu_uuid_generate(&uuid);
header.uuid_last_snap = uuid;
/* There is no need to set header.uuid_link or header.uuid_parent here. */
if (VDI_DEBUG) {
vdi_header_print(&header);
Expand Down

0 comments on commit ac928b8

Please sign in to comment.