From ebee92b4fef9defa19a8c348ec8b2716732ad4df Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:16:54 +0200 Subject: [PATCH 01/17] qemu-img: Plug memory leak on block option help error path Introduced in commit a283cb6; mostly harmless. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- qemu-img.c | 1 + 1 file changed, 1 insertion(+) diff --git a/qemu-img.c b/qemu-img.c index 1ad899e03b..62ea27eae5 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -287,6 +287,7 @@ static int print_block_option_help(const char *filename, const char *fmt) proto_drv = bdrv_find_protocol(filename, true); if (!proto_drv) { error_report("Unknown protocol '%s'", filename); + free_option_parameters(create_options); return 1; } create_options = append_option_parameters(create_options, From 75e347d66ab81944b5b657d17cc90ef92af3f016 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:16:55 +0200 Subject: [PATCH 02/17] block/vvfat: Plug memory leak in enable_write_target() I figure the leak originated in bdrv_create2(), and was duplicated into callers when commit 91a073a dropped that function. Looks like the other places have since been fixed. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block/vvfat.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/vvfat.c b/block/vvfat.c index 8f5114bd16..811b39c1ca 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2929,6 +2929,7 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp) set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:"); ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, errp); + free_option_parameters(options); if (ret < 0) { goto err; } From a1904e48c4a9fb114d155419700bfb7d760273b9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:16:56 +0200 Subject: [PATCH 03/17] qcow2: Plug memory leak on qcow2_invalidate_cache() error paths Introduced in commit 5a8a30d. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block/qcow2.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index a4b97e8263..a54d2ba897 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1308,6 +1308,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) options = qdict_clone_shallow(bs->options); ret = qcow2_open(bs, options, flags, &local_err); + QDECREF(options); if (local_err) { error_setg(errp, "Could not reopen qcow2 layer: %s", error_get_pretty(local_err)); @@ -1318,8 +1319,6 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) return; } - QDECREF(options); - if (crypt_method) { s->crypt_method = crypt_method; memcpy(&s->aes_encrypt_key, &aes_encrypt_key, sizeof(aes_encrypt_key)); From b20e61e0d52eef57cf5db55087b16e0b5207e730 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:16:57 +0200 Subject: [PATCH 04/17] block: Plug memory leak on brv_open_image() error path Introduced in commit da557a. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block.c b/block.c index a517d72d71..310ea89fce 100644 --- a/block.c +++ b/block.c @@ -1228,6 +1228,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, bdref_key); ret = -EINVAL; } + QDECREF(image_options); goto done; } From 443422fde7cb8410849074181de7b91bfd13b19d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:16:58 +0200 Subject: [PATCH 05/17] qemu-io: Support multiple -o in open command Instead of ignoring all option values but the last one, multiple -o options now have the same meaning as having a single option with all settings in the order of their respective -o options. Same as commit 2dc8328 for qemu-img convert, except here we do it with QemuOpts rather than QEMUOptionParameter. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- qemu-io.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/qemu-io.c b/qemu-io.c index 9fcd72bb10..ef3fef6e5f 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -118,6 +118,7 @@ static const cmdinfo_t open_cmd = { static QemuOptsList empty_opts = { .name = "drive", + .merge_lists = true, .head = QTAILQ_HEAD_INITIALIZER(empty_opts.head), .desc = { /* no elements => accept any params */ @@ -132,7 +133,7 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) int growable = 0; int c; QemuOpts *qopts; - QDict *opts = NULL; + QDict *opts; while ((c = getopt(argc, argv, "snrgo:")) != EOF) { switch (c) { @@ -149,15 +150,14 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) growable = 1; break; case 'o': - qopts = qemu_opts_parse(&empty_opts, optarg, 0); - if (qopts == NULL) { + if (!qemu_opts_parse(&empty_opts, optarg, 0)) { printf("could not parse option list -- %s\n", optarg); + qemu_opts_reset(&empty_opts); return 0; } - opts = qemu_opts_to_qdict(qopts, opts); - qemu_opts_del(qopts); break; default: + qemu_opts_reset(&empty_opts); return qemuio_command_usage(&open_cmd); } } @@ -166,6 +166,10 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) flags |= BDRV_O_RDWR; } + qopts = qemu_opts_find(&empty_opts, NULL); + opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL; + qemu_opts_reset(&empty_opts); + if (optind == argc - 1) { return openfile(argv[optind], flags, growable, opts); } else if (optind == argc) { From 29f2601aa605f0af0cba8eedcff7812c6c8532e9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:16:59 +0200 Subject: [PATCH 06/17] qemu-io: Plug memory leak in open command Introduced in commit b543c5c. Spotted by Coverity. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- qemu-io.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qemu-io.c b/qemu-io.c index ef3fef6e5f..f63e771d98 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -54,6 +54,7 @@ static int openfile(char *name, int flags, int growable, QDict *opts) if (qemuio_bs) { fprintf(stderr, "file open already, try 'help close'\n"); + QDECREF(opts); return 1; } @@ -175,6 +176,7 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) } else if (optind == argc) { return openfile(NULL, flags, growable, opts); } else { + QDECREF(opts); return qemuio_command_usage(&open_cmd); } } From 543f7bef1353be4b30543aa1888048a7b6abb2e9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:17:00 +0200 Subject: [PATCH 07/17] qemu-io: Don't print NULL when open without non-option arg fails Reproducer: "open -o a=b". Broken in commit fd0fee3. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- qemu-io.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qemu-io.c b/qemu-io.c index f63e771d98..795cf46c6e 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -62,7 +62,8 @@ static int openfile(char *name, int flags, int growable, QDict *opts) if (bdrv_open(&qemuio_bs, name, NULL, opts, flags | BDRV_O_PROTOCOL, NULL, &local_err)) { - fprintf(stderr, "%s: can't open device %s: %s\n", progname, name, + fprintf(stderr, "%s: can't open%s%s: %s\n", progname, + name ? " device " : "", name ?: "", error_get_pretty(local_err)); error_free(local_err); return 1; @@ -73,7 +74,8 @@ static int openfile(char *name, int flags, int growable, QDict *opts) if (bdrv_open(&qemuio_bs, name, NULL, opts, flags, NULL, &local_err) < 0) { - fprintf(stderr, "%s: can't open device %s: %s\n", progname, name, + fprintf(stderr, "%s: can't open%s%s: %s\n", progname, + name ? " device " : "", name ?: "", error_get_pretty(local_err)); error_free(local_err); bdrv_unref(qemuio_bs); From 6376f9522372d589f3efe60001dc0486237dd375 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:17:01 +0200 Subject: [PATCH 08/17] blockdev: Plug memory leak in blockdev_init() blockdev_init() leaks bs_opts when qemu_opts_create() fails, i.e. when the ID is bad. Missed in commit ec9c10d. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- blockdev.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index 8cc42fb1d0..652a0523b8 100644 --- a/blockdev.c +++ b/blockdev.c @@ -351,7 +351,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, opts = qemu_opts_create(&qemu_common_drive_opts, id, 1, &error); if (error) { error_propagate(errp, error); - return NULL; + goto err_no_opts; } qemu_opts_absorb_qdict(opts, bs_opts, &error); @@ -564,8 +564,9 @@ bdrv_new_err: g_free(dinfo->id); g_free(dinfo); early_err: - QDECREF(bs_opts); qemu_opts_del(opts); +err_no_opts: + QDECREF(bs_opts); return NULL; } From 3cb0e25c4b417b7336816bd92de458f0770d49ff Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:17:02 +0200 Subject: [PATCH 09/17] blockdev: Plug memory leak in drive_init() bs_opts is leaked on all paths from its qdev_new() that don't got through blockdev_init(). Add the missing QDECREF(), and zap bs_opts after blockdev_init(), so the new QDECREF() does nothing when we go through blockdev_init(). Leak introduced in commit f298d07. Spotted by Coverity. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- blockdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/blockdev.c b/blockdev.c index 652a0523b8..9b5261b765 100644 --- a/blockdev.c +++ b/blockdev.c @@ -940,6 +940,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) /* Actual block device init: Functionality shared with blockdev-add */ dinfo = blockdev_init(filename, bs_opts, &local_err); + bs_opts = NULL; if (dinfo == NULL) { if (local_err) { error_report("%s", error_get_pretty(local_err)); @@ -977,6 +978,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) fail: qemu_opts_del(legacy_opts); + QDECREF(bs_opts); return dinfo; } From f25391c2a6ef1674384204265429520ea50e82bc Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:17:03 +0200 Subject: [PATCH 10/17] block/qapi: Plug memory leak in dump_qobject() case QTYPE_QERROR Introduced in commit a8d8ecb. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block/qapi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/qapi.c b/block/qapi.c index 75f44f1537..97e16418ef 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -475,6 +475,7 @@ static void dump_qobject(fprintf_function func_fprintf, void *f, case QTYPE_QERROR: { QString *value = qerror_human((QError *)obj); func_fprintf(f, "%s", qstring_get_str(value)); + QDECREF(value); break; } case QTYPE_NONE: From 6262bbd363b53a1f19c473345d7cc40254dd5c73 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:17:04 +0200 Subject: [PATCH 11/17] block/vvfat: Plug memory leak in check_directory_consistency() On error path. Introduced in commit a046433a. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block/vvfat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/vvfat.c b/block/vvfat.c index 811b39c1ca..56370c5c93 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1866,7 +1866,7 @@ static int check_directory_consistency(BDRVVVFATState *s, if (s->used_clusters[cluster_num] & USED_ANY) { fprintf(stderr, "cluster %d used more than once\n", (int)cluster_num); - return 0; + goto fail; } s->used_clusters[cluster_num] = USED_DIRECTORY; From b122c3b6d020e529b203836efb8f611ece787293 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:17:05 +0200 Subject: [PATCH 12/17] block/vvfat: Plug memory leak in read_directory() Has always been leaky. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block/vvfat.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/vvfat.c b/block/vvfat.c index 56370c5c93..3cda19f2f3 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -787,7 +787,9 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) s->current_mapping->path=buffer; s->current_mapping->read_only = (st.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH)) == 0; - } + } else { + g_free(buffer); + } } closedir(dir); From 2df5fee2dbd56a9c34afd6d7df6744da2d951ccb Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:17:06 +0200 Subject: [PATCH 13/17] block/sheepdog: Plug memory leak in sd_snapshot_create() Has always been leaky. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block/sheepdog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 39f746157a..4ecbf5f498 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -2176,6 +2176,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) strncpy(s->inode.tag, sn_info->name, sizeof(s->inode.tag)); /* we don't need to update entire object */ datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id); + inode = g_malloc(datalen); /* refresh inode. */ fd = connect_to_sdog(s, &local_err); @@ -2202,8 +2203,6 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) goto cleanup; } - inode = (SheepdogInode *)g_malloc(datalen); - ret = read_object(fd, (char *)inode, vid_to_vdi_oid(new_vid), s->inode.nr_copies, datalen, 0, s->cache_flags); @@ -2217,6 +2216,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) s->inode.name, s->inode.snap_id, s->inode.vdi_id); cleanup: + g_free(inode); closesocket(fd); return ret; } From bb9cd2ee99f6537c072d5f4bac441717d3cd2bed Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 28 May 2014 11:17:07 +0200 Subject: [PATCH 14/17] qemu-img: Plug memory leak in convert command Introduced in commit 661a0f7. Spotted by Coverity. Signed-off-by: Markus Armbruster Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- qemu-img.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index 62ea27eae5..d118da5c2f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1455,7 +1455,7 @@ static int img_convert(int argc, char **argv) ret = bdrv_parse_cache_flags(cache, &flags); if (ret < 0) { error_report("Invalid cache option: %s", cache); - return -1; + goto out; } out_bs = bdrv_new_open("target", out_filename, out_fmt, flags, true, quiet); From 675036e4dadd2426c93c54d7f7155f519eda53e6 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 23 May 2014 17:15:41 +0100 Subject: [PATCH 15/17] block/raw-posix.c: Avoid nonstandard LONG_LONG_MAX In the MacOSX specific code in raw-posix.c we use the define LONG_LONG_MAX. This is actually a non-standard pre-C99 define; switch to using the standard LLONG_MAX instead. This apparently fixes a compilation failure with certain compiler/OS versions (though it is unclear which). Reported-by: Peter Bartoli Signed-off-by: Peter Maydell Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/raw-posix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 6586a0c9e1..b7f0f2624b 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1192,7 +1192,7 @@ again: if (size == 0) #endif #if defined(__APPLE__) && defined(__MACH__) - size = LONG_LONG_MAX; + size = LLONG_MAX; #else size = lseek(fd, 0LL, SEEK_END); #endif From c13959c745a7e4965c94d19e3153d2c44459906d Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 28 May 2014 11:38:58 +0800 Subject: [PATCH 16/17] vmdk: Fix local_err in vmdk_create In vmdk_create and vmdk_create_extent, initialize local_err before using it, and don't leak it on error. Reported-by: Markus Armbruster Signed-off-by: Fam Zheng Reviewed-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block/vmdk.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 480ea37d7c..2b38f61fcd 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1534,7 +1534,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, int ret, i; BlockDriverState *bs = NULL; VMDK4Header header; - Error *local_err; + Error *local_err = NULL; uint32_t tmp, magic, grains, gd_sectors, gt_size, gt_count; uint32_t *gd_buf = NULL; int gd_buf_size; @@ -1700,7 +1700,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, { int idx = 0; BlockDriverState *new_bs = NULL; - Error *local_err; + Error *local_err = NULL; char *desc = NULL; int64_t total_size = 0, filesize; const char *adapter_type = NULL; @@ -1881,7 +1881,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, } else { ret = bdrv_create_file(filename, options, &local_err); if (ret < 0) { - error_setg_errno(errp, -ret, "Could not create image file"); + error_propagate(errp, local_err); goto exit; } } @@ -1889,7 +1889,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, ret = bdrv_open(&new_bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err); if (ret < 0) { - error_setg_errno(errp, -ret, "Could not write description"); + error_propagate(errp, local_err); goto exit; } ret = bdrv_pwrite(new_bs, desc_offset, desc, desc_len); From 55d492d7602c27cabb605f42e72c755de1c186c1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Sat, 31 May 2014 21:33:30 +0200 Subject: [PATCH 17/17] qemu-img: Report error even with --oformat=json img_check() should report that the format of the given image does not support checks even if JSON output is desired. JSON data is output to stdout, as opposed to error messages, which are (in the case of qemu-img) printed to stderr. Therefore, it is easy to distinguish between the two. Also, img_info() does already use error_report() for human-readable messages even though JSON output is desired (through collect_image_info_list()). Signed-off-by: Max Reitz Reviewed-by: Markus Armbruster Signed-off-by: Kevin Wolf --- qemu-img.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index d118da5c2f..b3d2bc6f02 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -663,9 +663,7 @@ static int img_check(int argc, char **argv) ret = collect_image_check(bs, check, filename, fmt, fix); if (ret == -ENOTSUP) { - if (output_format == OFORMAT_HUMAN) { - error_report("This image format does not support checks"); - } + error_report("This image format does not support checks"); ret = 63; goto fail; }