From 6906046169ffa9d829beeeaafe1fadeba51669fb Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Tue, 13 May 2014 10:00:52 -0400 Subject: block: vhdx - account for identical header sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The VHDX spec v1.00 declares that "a header is current if it is the only valid header or if it is valid and its SequenceNumber field is greater than the other header’s SequenceNumber field. The parser must only use data from the current header. If there is no current header, then the VHDX file is corrupt." However, the Disk2VHD tool from Microsoft creates a VHDX image file that has 2 identical headers, including matching checksums and matching sequence numbers. Likely, as a shortcut the tool is just writing the header twice, for the active and inactive headers, during the image creation. Technically, this should be considered a corrupt VHDX file (at least per the 1.00 spec, and that is how we currently treat it). But in order to accomodate images created with Disk2VHD, we can safely create an exception for this case. If we find identical sequence numbers, then we check the VHDXHeader-sized chunks of each 64KB header sections (we won't rely just on the crc32c to indicate the headers are the same). If they are identical, then we go ahead and use the first one. Reported-by: Nerijus Baliūnas Signed-off-by: Jeff Cody Signed-off-by: Kevin Wolf --- block/vhdx.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'block') diff --git a/block/vhdx.c b/block/vhdx.c index 509baaf..353c74d 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -473,7 +473,14 @@ static void vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s, } else if (h2_seq > h1_seq) { s->curr_header = 1; } else { - goto fail; + /* The Microsoft Disk2VHD tool will create 2 identical + * headers, with identical sequence numbers. If the headers are + * identical, don't consider the file corrupt */ + if (!memcmp(header1, header2, sizeof(VHDXHeader))) { + s->curr_header = 0; + } else { + goto fail; + } } } -- cgit v1.1 From 9aedd5a5d607216e41bfa6a2c1f7f5d2b17041c3 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Wed, 14 May 2014 19:28:40 -0400 Subject: curl: Fix build when curl_multi_socket_action isn't available Signed-off-by: Matthew Booth Signed-off-by: Kevin Wolf --- block/curl.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'block') diff --git a/block/curl.c b/block/curl.c index d2f1084..f3c797a 100644 --- a/block/curl.c +++ b/block/curl.c @@ -37,6 +37,21 @@ #if LIBCURL_VERSION_NUM >= 0x071000 /* The multi interface timer callback was introduced in 7.16.0 */ #define NEED_CURL_TIMER_CALLBACK +#define HAVE_SOCKET_ACTION +#endif + +#ifndef HAVE_SOCKET_ACTION +/* If curl_multi_socket_action isn't available, define it statically here in + * terms of curl_multi_socket. Note that ev_bitmask will be ignored, which is + * less efficient but still safe. */ +static CURLMcode __curl_multi_socket_action(CURLM *multi_handle, + curl_socket_t sockfd, + int ev_bitmask, + int *running_handles) +{ + return curl_multi_socket(multi_handle, sockfd, running_handles); +} +#define curl_multi_socket_action __curl_multi_socket_action #endif #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \ -- cgit v1.1 From e3542c67af4cb4fd90e3be912630be9acd97b9c0 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Wed, 14 May 2014 19:28:41 -0400 Subject: curl: Remove broken parsing of options from url The block layer now supports a generic json syntax for passing option parameters explicitly, making parsing of options from the url redundant. Signed-off-by: Matthew Booth Signed-off-by: Kevin Wolf --- block/curl.c | 52 ++++++++++------------------------------------------ 1 file changed, 10 insertions(+), 42 deletions(-) (limited to 'block') diff --git a/block/curl.c b/block/curl.c index f3c797a..1b9f2f2 100644 --- a/block/curl.c +++ b/block/curl.c @@ -61,12 +61,15 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle, #define CURL_NUM_STATES 8 #define CURL_NUM_ACB 8 #define SECTOR_SIZE 512 -#define READ_AHEAD_SIZE (256 * 1024) +#define READ_AHEAD_DEFAULT (256 * 1024) #define FIND_RET_NONE 0 #define FIND_RET_OK 1 #define FIND_RET_WAIT 2 +#define CURL_BLOCK_OPT_URL "url" +#define CURL_BLOCK_OPT_READAHEAD "readahead" + struct BDRVCURLState; typedef struct CURLAIOCB { @@ -411,43 +414,7 @@ static void curl_clean_state(CURLState *s) static void curl_parse_filename(const char *filename, QDict *options, Error **errp) { - - #define RA_OPTSTR ":readahead=" - char *file; - char *ra; - const char *ra_val; - int parse_state = 0; - - file = g_strdup(filename); - - /* Parse a trailing ":readahead=#:" param, if present. */ - ra = file + strlen(file) - 1; - while (ra >= file) { - if (parse_state == 0) { - if (*ra == ':') { - parse_state++; - } else { - break; - } - } else if (parse_state == 1) { - if (*ra > '9' || *ra < '0') { - char *opt_start = ra - strlen(RA_OPTSTR) + 1; - if (opt_start > file && - strncmp(opt_start, RA_OPTSTR, strlen(RA_OPTSTR)) == 0) { - ra_val = ra + 1; - ra -= strlen(RA_OPTSTR) - 1; - *ra = '\0'; - qdict_put(options, "readahead", qstring_from_str(ra_val)); - } - break; - } - } - ra--; - } - - qdict_put(options, "url", qstring_from_str(file)); - - g_free(file); + qdict_put(options, CURL_BLOCK_OPT_URL, qstring_from_str(filename)); } static QemuOptsList runtime_opts = { @@ -455,12 +422,12 @@ static QemuOptsList runtime_opts = { .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), .desc = { { - .name = "url", + .name = CURL_BLOCK_OPT_URL, .type = QEMU_OPT_STRING, .help = "URL to open", }, { - .name = "readahead", + .name = CURL_BLOCK_OPT_READAHEAD, .type = QEMU_OPT_SIZE, .help = "Readahead size", }, @@ -492,14 +459,15 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, goto out_noclean; } - s->readahead_size = qemu_opt_get_size(opts, "readahead", READ_AHEAD_SIZE); + s->readahead_size = qemu_opt_get_size(opts, CURL_BLOCK_OPT_READAHEAD, + READ_AHEAD_DEFAULT); if ((s->readahead_size & 0x1ff) != 0) { error_setg(errp, "HTTP_READAHEAD_SIZE %zd is not a multiple of 512", s->readahead_size); goto out_noclean; } - file = qemu_opt_get(opts, "url"); + file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL); if (file == NULL) { error_setg(errp, "curl block driver requires an 'url' option"); goto out_noclean; -- cgit v1.1 From 97a3ea57198b39b8366cd2a7514707abdcd0a7bc Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Wed, 14 May 2014 19:28:42 -0400 Subject: curl: Add sslverify option This allows qemu to use images over https with a self-signed certificate. It defaults to verifying the certificate. Signed-off-by: Matthew Booth Signed-off-by: Kevin Wolf --- block/curl.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'block') diff --git a/block/curl.c b/block/curl.c index 1b9f2f2..f491b0b 100644 --- a/block/curl.c +++ b/block/curl.c @@ -23,6 +23,7 @@ */ #include "qemu-common.h" #include "block/block_int.h" +#include "qapi/qmp/qbool.h" #include // #define DEBUG @@ -69,6 +70,7 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle, #define CURL_BLOCK_OPT_URL "url" #define CURL_BLOCK_OPT_READAHEAD "readahead" +#define CURL_BLOCK_OPT_SSLVERIFY "sslverify" struct BDRVCURLState; @@ -106,6 +108,7 @@ typedef struct BDRVCURLState { CURLState states[CURL_NUM_STATES]; char *url; size_t readahead_size; + bool sslverify; bool accept_range; } BDRVCURLState; @@ -372,6 +375,8 @@ static CURLState *curl_init_state(BDRVCURLState *s) return NULL; } curl_easy_setopt(state->curl, CURLOPT_URL, s->url); + curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER, + (long) s->sslverify); curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, 5); curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, (void *)curl_read_cb); @@ -431,6 +436,11 @@ static QemuOptsList runtime_opts = { .type = QEMU_OPT_SIZE, .help = "Readahead size", }, + { + .name = CURL_BLOCK_OPT_SSLVERIFY, + .type = QEMU_OPT_BOOL, + .help = "Verify SSL certificate" + }, { /* end of list */ } }, }; @@ -467,6 +477,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, goto out_noclean; } + s->sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true); + file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL); if (file == NULL) { error_setg(errp, "curl block driver requires an 'url' option"); -- cgit v1.1 From ea54feff58efedc809641474b25a3130309678e7 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 7 May 2014 16:56:10 +0200 Subject: qcow1: Make padding in the header explicit We were relying on all compilers inserting the same padding in the header struct that is used for the on-disk format. Let's not do that. Mark the struct as packed and insert an explicit padding field for compatibility. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Benoit Canet --- block/qcow.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'block') diff --git a/block/qcow.c b/block/qcow.c index 937dd6d..3684794 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -48,9 +48,10 @@ typedef struct QCowHeader { uint64_t size; /* in bytes */ uint8_t cluster_bits; uint8_t l2_bits; + uint16_t padding; uint32_t crypt_method; uint64_t l1_table_offset; -} QCowHeader; +} QEMU_PACKED QCowHeader; #define L2_CACHE_SIZE 16 -- cgit v1.1 From 7159a45b2bf2dcb9f49f1e27d1d3d135a0247a2f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 7 May 2014 17:30:30 +0200 Subject: qcow1: Check maximum cluster size Huge values for header.cluster_bits cause unbounded allocations (e.g. for s->cluster_cache) and crash qemu this way. Less huge values may survive those allocations, but can cause integer overflows later on. The only cluster sizes that qemu can create are 4k (for standalone images) and 512 (for images with backing files), so we can limit it to 64k. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Benoit Canet --- block/qcow.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/qcow.c b/block/qcow.c index 3684794..e60df23 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -128,11 +128,17 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } - if (header.size <= 1 || header.cluster_bits < 9) { - error_setg(errp, "invalid value in qcow header"); + if (header.size <= 1) { + error_setg(errp, "Image size is too small (must be at least 2 bytes)"); ret = -EINVAL; goto fail; } + if (header.cluster_bits < 9 || header.cluster_bits > 16) { + error_setg(errp, "Cluster size must be between 512 and 64k"); + ret = -EINVAL; + goto fail; + } + if (header.crypt_method > QCOW_CRYPT_AES) { error_setg(errp, "invalid encryption method in qcow header"); ret = -EINVAL; -- cgit v1.1 From 42eb58179b3b215bb507da3262b682b8a2ec10b5 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 15 May 2014 16:10:11 +0200 Subject: qcow1: Validate L2 table size (CVE-2014-0222) Too large L2 table sizes cause unbounded allocations. Images actually created by qemu-img only have 512 byte or 4k L2 tables. To keep things consistent with cluster sizes, allow ranges between 512 bytes and 64k (in fact, down to 1 entry = 8 bytes is technically working, but L2 table sizes smaller than a cluster don't make a lot of sense). This also means that the number of bytes on the virtual disk that are described by the same L2 table is limited to at most 8k * 64k or 2^29, preventively avoiding any integer overflows. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Benoit Canet --- block/qcow.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'block') diff --git a/block/qcow.c b/block/qcow.c index e60df23..e8038e5 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -139,6 +139,14 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } + /* l2_bits specifies number of entries; storing a uint64_t in each entry, + * so bytes = num_entries << 3. */ + if (header.l2_bits < 9 - 3 || header.l2_bits > 16 - 3) { + error_setg(errp, "L2 table size must be between 512 and 64k"); + ret = -EINVAL; + goto fail; + } + if (header.crypt_method > QCOW_CRYPT_AES) { error_setg(errp, "invalid encryption method in qcow header"); ret = -EINVAL; -- cgit v1.1 From 46485de0cb357b57373e1ca895adedf1f3ed46ec Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 8 May 2014 13:08:20 +0200 Subject: qcow1: Validate image size (CVE-2014-0223) A huge image size could cause s->l1_size to overflow. Make sure that images never require a L1 table larger than what fits in s->l1_size. This cannot only cause unbounded allocations, but also the allocation of a too small L1 table, resulting in out-of-bounds array accesses (both reads and writes). Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf --- block/qcow.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/qcow.c b/block/qcow.c index e8038e5..3566c05 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -61,7 +61,7 @@ typedef struct BDRVQcowState { int cluster_sectors; int l2_bits; int l2_size; - int l1_size; + unsigned int l1_size; uint64_t cluster_offset_mask; uint64_t l1_table_offset; uint64_t *l1_table; @@ -166,7 +166,19 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, /* read the level 1 table */ shift = s->cluster_bits + s->l2_bits; - s->l1_size = (header.size + (1LL << shift) - 1) >> shift; + if (header.size > UINT64_MAX - (1LL << shift)) { + error_setg(errp, "Image too large"); + ret = -EINVAL; + goto fail; + } else { + uint64_t l1_size = (header.size + (1LL << shift) - 1) >> shift; + if (l1_size > INT_MAX / sizeof(uint64_t)) { + error_setg(errp, "Image too large"); + ret = -EINVAL; + goto fail; + } + s->l1_size = l1_size; + } s->l1_table_offset = header.l1_table_offset; s->l1_table = g_malloc(s->l1_size * sizeof(uint64_t)); -- cgit v1.1 From d66e5cee002c471b78139228a4e7012736b375f9 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 8 May 2014 13:35:09 +0200 Subject: qcow1: Stricter backing file length check Like qcow2 since commit 6d33e8e7, error out on invalid lengths instead of silently truncating them to 1023. Also don't rely on bdrv_pread() catching integer overflows that make len negative, but use unsigned variables in the first place. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Benoit Canet --- block/qcow.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/qcow.c b/block/qcow.c index 3566c05..7fd57d7 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -97,7 +97,8 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVQcowState *s = bs->opaque; - int len, i, shift, ret; + unsigned int len, i, shift; + int ret; QCowHeader header; ret = bdrv_pread(bs->file, 0, &header, sizeof(header)); @@ -202,7 +203,9 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, if (header.backing_file_offset != 0) { len = header.backing_file_size; if (len > 1023) { - len = 1023; + error_setg(errp, "Backing file name too long"); + ret = -EINVAL; + goto fail; } ret = bdrv_pread(bs->file, header.backing_file_offset, bs->backing_file, len); -- cgit v1.1 From 465bee1da82e43f18d10c43cc7566d0284ad13a9 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Sun, 18 May 2014 00:58:19 +0200 Subject: block: optimize zero writes with bdrv_write_zeroes this patch tries to optimize zero write requests by automatically using bdrv_write_zeroes if it is supported by the format. This significantly speeds up file system initialization and should speed zero write test used to test backend storage performance. I ran the following 2 tests on my internal SSD with a 50G QCOW2 container and on an attached iSCSI storage. a) mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 /dev/vdX QCOW2 [off] [on] [unmap] ----- runtime: 14secs 1.1secs 1.1secs filesize: 937M 18M 18M iSCSI [off] [on] [unmap] ---- runtime: 9.3s 0.9s 0.9s b) dd if=/dev/zero of=/dev/vdX bs=1M oflag=direct QCOW2 [off] [on] [unmap] ----- runtime: 246secs 18secs 18secs filesize: 51G 192K 192K throughput: 203M/s 2.3G/s 2.3G/s iSCSI* [off] [on] [unmap] ---- runtime: 8mins 45secs 33secs throughput: 106M/s 1.2G/s 1.6G/s allocated: 100% 100% 0% * The storage was connected via an 1Gbit interface. It seems to internally handle writing zeroes via WRITESAME16 very fast. Signed-off-by: Peter Lieven Signed-off-by: Kevin Wolf --- block/qapi.c | 1 + 1 file changed, 1 insertion(+) (limited to 'block') diff --git a/block/qapi.c b/block/qapi.c index af11445..75f44f1 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -50,6 +50,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs) } info->backing_file_depth = bdrv_get_backing_file_depth(bs); + info->detect_zeroes = bs->detect_zeroes; if (bs->io_limits_enabled) { ThrottleConfig cfg; -- cgit v1.1