From fc11eb26cee7e3621645dd40cd9de944201f590b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 5 Aug 2013 10:53:04 +0200 Subject: [PATCH 01/14] qemu-img: Error out for excess arguments Don't silently ignore excess arguments at the end of the command line, but error out instead. This can catch typos like 'resize test.img + 1G', which doesn't increase the image size by 1G as intended, but truncates the image to 1G. Even for less dangerous commands, the old behaviour is confusing. Signed-off-by: Kevin Wolf Reviewed-by: Laszlo Ersek Reviewed-by: Stefan Hajnoczi --- qemu-img.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index c55ca5c557..dece1b3a3a 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -396,6 +396,9 @@ static int img_create(int argc, char **argv) } img_size = (uint64_t)sval; } + if (optind != argc) { + help(); + } if (options && is_help_option(options)) { return print_block_option_help(filename, fmt); @@ -573,7 +576,7 @@ static int img_check(int argc, char **argv) break; } } - if (optind >= argc) { + if (optind != argc - 1) { help(); } filename = argv[optind++]; @@ -684,7 +687,7 @@ static int img_commit(int argc, char **argv) break; } } - if (optind >= argc) { + if (optind != argc - 1) { help(); } filename = argv[optind++]; @@ -930,7 +933,7 @@ static int img_compare(int argc, char **argv) } - if (optind > argc - 2) { + if (optind != argc - 2) { help(); } filename1 = argv[optind++]; @@ -1741,7 +1744,7 @@ static int img_info(int argc, char **argv) break; } } - if (optind >= argc) { + if (optind != argc - 1) { help(); } filename = argv[optind++]; @@ -1842,7 +1845,7 @@ static int img_snapshot(int argc, char **argv) } } - if (optind >= argc) { + if (optind != argc - 1) { help(); } filename = argv[optind++]; @@ -1953,7 +1956,7 @@ static int img_rebase(int argc, char **argv) progress = 0; } - if ((optind >= argc) || (!unsafe && !out_baseimg)) { + if ((optind != argc - 1) || (!unsafe && !out_baseimg)) { help(); } filename = argv[optind++]; @@ -2232,7 +2235,7 @@ static int img_resize(int argc, char **argv) break; } } - if (optind >= argc) { + if (optind != argc - 1) { help(); } filename = argv[optind++]; From 526eda14a68d5b3596be715505289b541288ef2a Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Tue, 23 Jul 2013 17:30:11 +0900 Subject: [PATCH 02/14] ignore SIGPIPE in qemu-img and qemu-io This prevents the tools from being stopped when they write data to a closed connection in the other side. Signed-off-by: MORITA Kazutaka Reviewed-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- qemu-img.c | 4 ++++ qemu-io.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/qemu-img.c b/qemu-img.c index dece1b3a3a..b9a848db74 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2322,6 +2322,10 @@ int main(int argc, char **argv) const img_cmd_t *cmd; const char *cmdname; +#ifdef CONFIG_POSIX + signal(SIGPIPE, SIG_IGN); +#endif + error_set_progname(argv[0]); qemu_init_main_loop(); diff --git a/qemu-io.c b/qemu-io.c index cb9def50e6..d54dc86921 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -335,6 +335,10 @@ int main(int argc, char **argv) int opt_index = 0; int flags = BDRV_O_UNMAP; +#ifdef CONFIG_POSIX + signal(SIGPIPE, SIG_IGN); +#endif + progname = basename(argv[0]); while ((c = getopt_long(argc, argv, sopt, lopt, &opt_index)) != -1) { From 840042901710c2dc1a3ac3e5af9bed449c339701 Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Tue, 23 Jul 2013 17:30:12 +0900 Subject: [PATCH 03/14] iov: handle EOF in iov_send_recv Without this patch, iov_send_recv() never returns when do_send_recv() returns zero. Signed-off-by: MORITA Kazutaka Reviewed-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- util/iov.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/util/iov.c b/util/iov.c index cc6e837c83..f705586808 100644 --- a/util/iov.c +++ b/util/iov.c @@ -202,6 +202,12 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt, return -1; } + if (ret == 0 && !do_send) { + /* recv returns 0 when the peer has performed an orderly + * shutdown. */ + break; + } + /* Prepare for the next iteration */ offset += ret; total += ret; From 9580498b9a599b38c3a28599dcd40bd59f12af2c Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 5 Aug 2013 14:40:34 +0200 Subject: [PATCH 04/14] qemu-iotests: filter QEMU version in monitor banner Filter out the QEMU monitor version banner so that tests do not break when the QEMU version number is changed. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/051.out | 64 ++++++++++++++++---------------- tests/qemu-iotests/common.filter | 3 +- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 9588d0c977..5582ed3655 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -23,11 +23,11 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could not === Enable and disable lazy refcounting on the command line, plus some invalid values === Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=off -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts= @@ -51,72 +51,72 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: Lazy ref QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: could not open disk image TEST_DIR/t.qcow2: Invalid argument Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=off -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit === No medium === Testing: -drive if=floppy -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive if=ide,media=cdrom -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive if=scsi,media=cdrom -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive if=ide -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: Device needs media, but drive is empty QEMU_PROG: Device initialization failed. QEMU_PROG: Initialization of device ide-hd failed Testing: -drive if=virtio -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -drive if=virtio: Device needs media, but drive is empty QEMU_PROG: -drive if=virtio: Device initialization failed. QEMU_PROG: -drive if=virtio: Device initialization failed. QEMU_PROG: -drive if=virtio: Device 'virtio-blk-pci' could not be initialized Testing: -drive if=scsi -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -drive if=scsi: Device needs media, but drive is empty QEMU_PROG: -drive if=scsi: Device initialization failed. QEMU_PROG: Device initialization failed. QEMU_PROG: Initialization of device lsi53c895a failed Testing: -drive if=none,id=disk -device ide-cd,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive if=none,id=disk -device ide-drive,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty QEMU_PROG: -device ide-drive,drive=disk: Device initialization failed. QEMU_PROG: -device ide-drive,drive=disk: Device 'ide-drive' could not be initialized Testing: -drive if=none,id=disk -device ide-hd,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -device ide-hd,drive=disk: Device needs media, but drive is empty QEMU_PROG: -device ide-hd,drive=disk: Device initialization failed. QEMU_PROG: -device ide-hd,drive=disk: Device 'ide-hd' could not be initialized Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -device scsi-disk,drive=disk: Device needs media, but drive is empty QEMU_PROG: -device scsi-disk,drive=disk: Device initialization failed. QEMU_PROG: -device scsi-disk,drive=disk: Device 'scsi-disk' could not be initialized Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -device scsi-hd,drive=disk: Device needs media, but drive is empty QEMU_PROG: -device scsi-hd,drive=disk: Device initialization failed. QEMU_PROG: -device scsi-hd,drive=disk: Device 'scsi-hd' could not be initialized @@ -125,77 +125,77 @@ QEMU_PROG: -device scsi-hd,drive=disk: Device 'scsi-hd' could not be initialized === Read-only === Testing: -drive file=TEST_DIR/t.qcow2,if=floppy,readonly=on -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=ide,media=cdrom,readonly=on -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,media=cdrom,readonly=on -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on: read-only not supported by this bus type Testing: -drive file=TEST_DIR/t.qcow2,if=virtio,readonly=on -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,readonly=on -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-cd,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-cd,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-drive,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -device ide-drive,drive=disk: Can't use a read-only drive QEMU_PROG: -device ide-drive,drive=disk: Device initialization failed. QEMU_PROG: -device ide-drive,drive=disk: Device 'ide-drive' could not be initialized Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-hd,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -device ide-hd,drive=disk: Can't use a read-only drive QEMU_PROG: -device ide-hd,drive=disk: Device initialization failed. QEMU_PROG: -device ide-hd,drive=disk: Device 'ide-hd' could not be initialized Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit === Cache modes === Testing: -drive media=cdrom,cache=none -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive media=cdrom,cache=directsync -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive media=cdrom,cache=writeback -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive media=cdrom,cache=writethrough -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive media=cdrom,cache=unsafe -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive media=cdrom,cache=invalid_value @@ -205,7 +205,7 @@ QEMU_PROG: -drive media=cdrom,cache=invalid_value: invalid cache option === Specifying the protocol layer === Testing: -drive file=TEST_DIR/t.qcow2,file.driver=file -QEMU 1.5.50 monitor - type 'help' for more information +QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2 diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 9dbcae8d8c..97a31ff0b1 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -155,7 +155,8 @@ _filter_qemu_io() # replace occurrences of QEMU_PROG with "qemu" _filter_qemu() { - sed -e "s#\\(^\\|(qemu) \\)$(basename $QEMU_PROG):#\1QEMU_PROG:#" + sed -e "s#\\(^\\|(qemu) \\)$(basename $QEMU_PROG):#\1QEMU_PROG:#" \ + -e 's#^QEMU [0-9]\+\.[0-9]\+\.[0-9]\+ monitor#QEMU X.Y.Z monitor#' } # make sure this script returns success From e4f5c1bf8f6f6fe0bb4c743452bf8288033e80ba Mon Sep 17 00:00:00 2001 From: Liu Yuan Date: Tue, 6 Aug 2013 14:44:37 +0800 Subject: [PATCH 05/14] sheepdog: add missing .bdrv_has_zero_init Commit 3ac21627 changed the behaviour of bdrv_has_zero_init() to default to 0. In the review for Sheepdog it turned out that enabling it is safe, so that commit updated one BlockDriver definition of sheepdog to use bdrv_has_zero_init_1, missed however that there are more BlockDrivers in the driver. Fix these now. Cc: Kevin Wolf Cc: Stefan Hajnoczi Signed-off-by: Liu Yuan Reviewed-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/sheepdog.c b/block/sheepdog.c index a506137bb8..afe053376c 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -2347,6 +2347,7 @@ static BlockDriver bdrv_sheepdog = { .bdrv_file_open = sd_open, .bdrv_close = sd_close, .bdrv_create = sd_create, + .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_getlength = sd_getlength, .bdrv_truncate = sd_truncate, @@ -2374,6 +2375,7 @@ static BlockDriver bdrv_sheepdog_tcp = { .bdrv_file_open = sd_open, .bdrv_close = sd_close, .bdrv_create = sd_create, + .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_getlength = sd_getlength, .bdrv_truncate = sd_truncate, From 5d8caa543c9714bee36b04899797a3721dff4090 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 6 Aug 2013 15:44:47 +0800 Subject: [PATCH 06/14] vmdk: Make VMDK3Header and VmdkGrainMarker QEMU_PACKED It's best to make it consistent that all on disk structures are QEMU_PACKED. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index e6c50b1e35..5c3c24086f 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -62,7 +62,7 @@ typedef struct { uint32_t cylinders; uint32_t heads; uint32_t sectors_per_track; -} VMDK3Header; +} QEMU_PACKED VMDK3Header; typedef struct { uint32_t version; @@ -131,7 +131,7 @@ typedef struct VmdkGrainMarker { uint64_t lba; uint32_t size; uint8_t data[0]; -} VmdkGrainMarker; +} QEMU_PACKED VmdkGrainMarker; enum { MARKER_END_OF_STREAM = 0, From e98768d43799cd3f00b358bfbe455fdae793d3e8 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 6 Aug 2013 15:44:48 +0800 Subject: [PATCH 07/14] vmdk: use unsigned values for on disk header fields The size and offset fields are all non-negative values, use uint64_t for them to avoid getting negative in memory value by int overflow. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 5c3c24086f..2c925da0aa 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -67,14 +67,14 @@ typedef struct { typedef struct { uint32_t version; uint32_t flags; - int64_t capacity; - int64_t granularity; - int64_t desc_offset; - int64_t desc_size; - int32_t num_gtes_per_gte; - int64_t rgd_offset; - int64_t gd_offset; - int64_t grain_offset; + uint64_t capacity; + uint64_t granularity; + uint64_t desc_offset; + uint64_t desc_size; + uint32_t num_gtes_per_gte; + uint64_t rgd_offset; + uint64_t gd_offset; + uint64_t grain_offset; char filler[1]; char check_bytes[4]; uint16_t compressAlgorithm; @@ -109,7 +109,7 @@ typedef struct VmdkExtent { typedef struct BDRVVmdkState { CoMutex lock; - int desc_offset; + uint64_t desc_offset; bool cid_updated; uint32_t parent_cid; int num_extents; @@ -490,7 +490,7 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, - int64_t desc_offset); + uint64_t desc_offset); static int vmdk_open_vmdk4(BlockDriverState *bs, BlockDriverState *file, @@ -508,7 +508,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, return ret; } if (header.capacity == 0) { - int64_t desc_offset = le64_to_cpu(header.desc_offset); + uint64_t desc_offset = le64_to_cpu(header.desc_offset); if (desc_offset) { return vmdk_open_desc_file(bs, flags, desc_offset << 9); } @@ -728,7 +728,7 @@ next_line: } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, - int64_t desc_offset) + uint64_t desc_offset) { int ret; char *buf = NULL; From 23ea2ecc2a43d850bc9482068201ece5da36a448 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 6 Aug 2013 15:44:49 +0800 Subject: [PATCH 08/14] qemu-iotests: add poke_file utility function The new poke_file function sets bytes at an offset in a file given a printf-style format string. It can be used to corrupt an image file for test coverage of error paths. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.rc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index e9ba3586b5..5e077c3573 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -34,6 +34,12 @@ dd() fi } +# poke_file 'test.img' 512 '\xff\xfe' +poke_file() +{ + printf "$3" | dd "of=$1" bs=1 "seek=$2" conv=notrunc &>/dev/null +} + # we need common.config if [ "$iam" != "check" ] then From ca6cbb657d66a7beb70f9d91848c80d1a72b1674 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 6 Aug 2013 15:44:50 +0800 Subject: [PATCH 09/14] qemu-iotests: add empty test case for vmdk Will add vmdk specific tests later here. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- tests/qemu-iotests/059 | 51 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/059.out | 2 ++ tests/qemu-iotests/group | 1 + 3 files changed, 54 insertions(+) create mode 100755 tests/qemu-iotests/059 create mode 100644 tests/qemu-iotests/059.out diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 new file mode 100755 index 0000000000..9dc7f64d79 --- /dev/null +++ b/tests/qemu-iotests/059 @@ -0,0 +1,51 @@ +#!/bin/bash +# +# Test case for vmdk +# +# Copyright (C) 2013 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=famz@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +# This tests vmdk-specific low-level functionality +_supported_fmt vmdk +_supported_proto generic +_supported_os Linux + +granularity_offset=16 + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out new file mode 100644 index 0000000000..4ca7f29722 --- /dev/null +++ b/tests/qemu-iotests/059.out @@ -0,0 +1,2 @@ +QA output created by 059 +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 69e208c709..43c05d6f5c 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -63,3 +63,4 @@ 054 rw auto 055 rw auto 056 rw auto backing +059 rw auto From 8aa1331c09a9b899f48d97f097bb49b7d458be1c Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 6 Aug 2013 15:44:51 +0800 Subject: [PATCH 10/14] vmdk: check granularity field in opening Granularity is used to calculate the cluster size and allocate r/w buffer. Check the value from image before using it, so we don't abort() for unbounded memory allocation. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 40 +++++++++++++++++++++++++++++--------- tests/qemu-iotests/059 | 8 +++++++- tests/qemu-iotests/059.out | 6 ++++++ 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 2c925da0aa..015cbd4e60 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -385,15 +385,22 @@ static int vmdk_parent_open(BlockDriverState *bs) /* Create and append extent to the extent array. Return the added VmdkExtent * address. return NULL if allocation failed. */ -static VmdkExtent *vmdk_add_extent(BlockDriverState *bs, +static int vmdk_add_extent(BlockDriverState *bs, BlockDriverState *file, bool flat, int64_t sectors, int64_t l1_offset, int64_t l1_backup_offset, uint32_t l1_size, - int l2_size, unsigned int cluster_sectors) + int l2_size, uint64_t cluster_sectors, + VmdkExtent **new_extent) { VmdkExtent *extent; BDRVVmdkState *s = bs->opaque; + if (cluster_sectors > 0x200000) { + /* 0x200000 * 512Bytes = 1GB for one cluster is unrealistic */ + error_report("invalid granularity, image may be corrupt"); + return -EINVAL; + } + s->extents = g_realloc(s->extents, (s->num_extents + 1) * sizeof(VmdkExtent)); extent = &s->extents[s->num_extents]; @@ -416,7 +423,10 @@ static VmdkExtent *vmdk_add_extent(BlockDriverState *bs, extent->end_sector = extent->sectors; } bs->total_sectors = extent->end_sector; - return extent; + if (new_extent) { + *new_extent = extent; + } + return 0; } static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) @@ -475,12 +485,17 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, if (ret < 0) { return ret; } - extent = vmdk_add_extent(bs, + + ret = vmdk_add_extent(bs, bs->file, false, le32_to_cpu(header.disk_sectors), le32_to_cpu(header.l1dir_offset) << 9, 0, 1 << 6, 1 << 9, - le32_to_cpu(header.granularity)); + le32_to_cpu(header.granularity), + &extent); + if (ret < 0) { + return ret; + } ret = vmdk_init_tables(bs, extent); if (ret) { /* free extent allocated by vmdk_add_extent */ @@ -580,13 +595,17 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, if (le32_to_cpu(header.flags) & VMDK4_FLAG_RGD) { l1_backup_offset = le64_to_cpu(header.rgd_offset) << 9; } - extent = vmdk_add_extent(bs, file, false, + ret = vmdk_add_extent(bs, file, false, le64_to_cpu(header.capacity), le64_to_cpu(header.gd_offset) << 9, l1_backup_offset, l1_size, le32_to_cpu(header.num_gtes_per_gte), - le64_to_cpu(header.granularity)); + le64_to_cpu(header.granularity), + &extent); + if (ret < 0) { + return ret; + } extent->compressed = le16_to_cpu(header.compressAlgorithm) == VMDK4_COMPRESSION_DEFLATE; extent->has_marker = le32_to_cpu(header.flags) & VMDK4_FLAG_MARKER; @@ -702,8 +721,11 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, /* FLAT extent */ VmdkExtent *extent; - extent = vmdk_add_extent(bs, extent_file, true, sectors, - 0, 0, 0, 0, sectors); + ret = vmdk_add_extent(bs, extent_file, true, sectors, + 0, 0, 0, 0, sectors, &extent); + if (ret < 0) { + return ret; + } extent->flat_start_offset = flat_offset << 9; } else if (!strcmp(type, "SPARSE")) { /* SPARSE extent */ diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index 9dc7f64d79..9545e82bc2 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -43,7 +43,13 @@ _supported_fmt vmdk _supported_proto generic _supported_os Linux -granularity_offset=16 +granularity_offset=20 + +echo "=== Testing invalid granularity ===" +echo +_make_test_img 64M +poke_file "$TEST_IMG" "$granularity_offset" "\xff\xff\xff\xff\xff\xff\xff\xff" +{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir # success, all done echo "*** done" diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 4ca7f29722..380ca3d943 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -1,2 +1,8 @@ QA output created by 059 +=== Testing invalid granularity === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +invalid granularity, image may be corrupt +qemu-io: can't open device TEST_DIR/t.vmdk +no file open, try 'help open' *** done From f8ce04036e333aae480b1d06d969f6436652633d Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 6 Aug 2013 15:44:52 +0800 Subject: [PATCH 11/14] vmdk: check l2 table size when opening header.num_gtes_per_gte determines size for L2 table. Check for too big value before using it. Limit to 512M entries (2GB per one L2 table). Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 5 +++++ tests/qemu-iotests/059 | 7 +++++++ tests/qemu-iotests/059.out | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/block/vmdk.c b/block/vmdk.c index 015cbd4e60..53020ef3e3 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -585,6 +585,11 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, return -ENOTSUP; } + if (le32_to_cpu(header.num_gtes_per_gte) > 512) { + error_report("L2 table size too big"); + return -EINVAL; + } + l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte) * le64_to_cpu(header.granularity); if (l1_entry_sectors == 0) { diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index 9545e82bc2..301eacaf7e 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -44,6 +44,7 @@ _supported_proto generic _supported_os Linux granularity_offset=20 +grain_table_size_offset=44 echo "=== Testing invalid granularity ===" echo @@ -51,6 +52,12 @@ _make_test_img 64M poke_file "$TEST_IMG" "$granularity_offset" "\xff\xff\xff\xff\xff\xff\xff\xff" { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir +echo "=== Testing too big L2 table size ===" +echo +_make_test_img 64M +poke_file "$TEST_IMG" "$grain_table_size_offset" "\xff\xff\xff\xff" +{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 380ca3d943..583955fc7d 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -5,4 +5,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 invalid granularity, image may be corrupt qemu-io: can't open device TEST_DIR/t.vmdk no file open, try 'help open' +=== Testing too big L2 table size === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +L2 table size too big +qemu-io: can't open device TEST_DIR/t.vmdk +no file open, try 'help open' *** done From 2c43e43c8cec130fff95ef720a860e91efb36685 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 6 Aug 2013 15:44:53 +0800 Subject: [PATCH 12/14] vmdk: check l1 size before opening image L1 table size is calculated from capacity, granularity and l2 table size. If capacity is too big or later two are too small, the L1 table will be too big to allocate in memory. Limit it to a reasonable range. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 8 ++++++++ tests/qemu-iotests/059 | 8 ++++++++ tests/qemu-iotests/059.out | 6 ++++++ 3 files changed, 22 insertions(+) diff --git a/block/vmdk.c b/block/vmdk.c index 53020ef3e3..955125a7f4 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -597,6 +597,14 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, } l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1) / l1_entry_sectors; + if (l1_size > 512 * 1024 * 1024) { + /* although with big capacity and small l1_entry_sectors, we can get a + * big l1_size, we don't want unbounded value to allocate the table. + * Limit it to 512M, which is 16PB for default cluster and L2 table + * size */ + error_report("L1 size too big"); + return -EFBIG; + } if (le32_to_cpu(header.flags) & VMDK4_FLAG_RGD) { l1_backup_offset = le64_to_cpu(header.rgd_offset) << 9; } diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index 301eacaf7e..b03429dd01 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -43,6 +43,7 @@ _supported_fmt vmdk _supported_proto generic _supported_os Linux +capacity_offset=16 granularity_offset=20 grain_table_size_offset=44 @@ -58,6 +59,13 @@ _make_test_img 64M poke_file "$TEST_IMG" "$grain_table_size_offset" "\xff\xff\xff\xff" { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir +echo "=== Testing too big L1 table size ===" +echo +_make_test_img 64M +poke_file "$TEST_IMG" "$capacity_offset" "\xff\xff\xff\xff" +poke_file "$TEST_IMG" "$grain_table_size_offset" "\x01\x00\x00\x00" +{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 583955fc7d..9e715e5a95 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -11,4 +11,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 L2 table size too big qemu-io: can't open device TEST_DIR/t.vmdk no file open, try 'help open' +=== Testing too big L1 table size === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +L1 size too big +qemu-io: can't open device TEST_DIR/t.vmdk +no file open, try 'help open' *** done From bf81507de38fdfa4cb6e9b46fb38691a25cb1499 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 6 Aug 2013 15:44:54 +0800 Subject: [PATCH 13/14] vmdk: use heap allocation for whole_grain We should never grow the stack beyond 1 MB, otherwise we'll fall off the end. Thread stacks and coroutine stacks (1 MB) do not grow. get_cluster_offset() allocates a big stack offset, it will fail for big cluster images, change to heap allocated buffer. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 955125a7f4..ad0a4f366e 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -842,16 +842,17 @@ static int get_whole_cluster(BlockDriverState *bs, uint64_t offset, bool allocate) { - /* 128 sectors * 512 bytes each = grain size 64KB */ - uint8_t whole_grain[extent->cluster_sectors * 512]; + int ret = VMDK_OK; + uint8_t *whole_grain = NULL; /* we will be here if it's first write on non-exist grain(cluster). * try to read from parent image, if exist */ if (bs->backing_hd) { - int ret; - + whole_grain = + qemu_blockalign(bs, extent->cluster_sectors << BDRV_SECTOR_BITS); if (!vmdk_is_cid_valid(bs)) { - return VMDK_ERROR; + ret = VMDK_ERROR; + goto exit; } /* floor offset to cluster */ @@ -859,17 +860,21 @@ static int get_whole_cluster(BlockDriverState *bs, ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain, extent->cluster_sectors); if (ret < 0) { - return VMDK_ERROR; + ret = VMDK_ERROR; + goto exit; } /* Write grain only into the active image */ ret = bdrv_write(extent->file, cluster_offset, whole_grain, extent->cluster_sectors); if (ret < 0) { - return VMDK_ERROR; + ret = VMDK_ERROR; + goto exit; } } - return VMDK_OK; +exit: + qemu_vfree(whole_grain); + return ret; } static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data) From ca8804ced9fdba7a1925ed81084dfb7a5b6ffa9f Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 6 Aug 2013 15:44:55 +0800 Subject: [PATCH 14/14] vmdk: rename num_gtes_per_gte to num_gtes_per_gt num_gtes_per_gte is a historical typo, rename it to a more sensible name. It means "number of GrainTableEntries per GrainTable". Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index ad0a4f366e..346bb5cad9 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -71,7 +71,8 @@ typedef struct { uint64_t granularity; uint64_t desc_offset; uint64_t desc_size; - uint32_t num_gtes_per_gte; + /* Number of GrainTableEntries per GrainTable */ + uint32_t num_gtes_per_gt; uint64_t rgd_offset; uint64_t gd_offset; uint64_t grain_offset; @@ -585,12 +586,12 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, return -ENOTSUP; } - if (le32_to_cpu(header.num_gtes_per_gte) > 512) { + if (le32_to_cpu(header.num_gtes_per_gt) > 512) { error_report("L2 table size too big"); return -EINVAL; } - l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte) + l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gt) * le64_to_cpu(header.granularity); if (l1_entry_sectors == 0) { return -EINVAL; @@ -613,7 +614,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, le64_to_cpu(header.gd_offset) << 9, l1_backup_offset, l1_size, - le32_to_cpu(header.num_gtes_per_gte), + le32_to_cpu(header.num_gtes_per_gt), le64_to_cpu(header.granularity), &extent); if (ret < 0) { @@ -1411,12 +1412,12 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, header.compressAlgorithm = compress ? VMDK4_COMPRESSION_DEFLATE : 0; header.capacity = filesize / 512; header.granularity = 128; - header.num_gtes_per_gte = 512; + header.num_gtes_per_gt = 512; grains = (filesize / 512 + header.granularity - 1) / header.granularity; - gt_size = ((header.num_gtes_per_gte * sizeof(uint32_t)) + 511) >> 9; + gt_size = ((header.num_gtes_per_gt * sizeof(uint32_t)) + 511) >> 9; gt_count = - (grains + header.num_gtes_per_gte - 1) / header.num_gtes_per_gte; + (grains + header.num_gtes_per_gt - 1) / header.num_gtes_per_gt; gd_size = (gt_count * sizeof(uint32_t) + 511) >> 9; header.desc_offset = 1; @@ -1432,7 +1433,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, header.flags = cpu_to_le32(header.flags); header.capacity = cpu_to_le64(header.capacity); header.granularity = cpu_to_le64(header.granularity); - header.num_gtes_per_gte = cpu_to_le32(header.num_gtes_per_gte); + header.num_gtes_per_gt = cpu_to_le32(header.num_gtes_per_gt); header.desc_offset = cpu_to_le64(header.desc_offset); header.desc_size = cpu_to_le64(header.desc_size); header.rgd_offset = cpu_to_le64(header.rgd_offset);