From nobody Wed May 14 16:53:56 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1522407570864317.3971584803593; Fri, 30 Mar 2018 03:59:30 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3A96A7E9C1; Fri, 30 Mar 2018 10:59:29 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 11F885C1A1; Fri, 30 Mar 2018 10:59:29 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 13E904CA97; Fri, 30 Mar 2018 10:59:28 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w2UAxO7P011038 for ; Fri, 30 Mar 2018 06:59:24 -0400 Received: by smtp.corp.redhat.com (Postfix) id 6A7D063537; Fri, 30 Mar 2018 10:59:24 +0000 (UTC) Received: from angien.brq.redhat.com (unknown [10.43.2.136]) by smtp.corp.redhat.com (Postfix) with ESMTP id CAA4084436; Fri, 30 Mar 2018 10:59:23 +0000 (UTC) From: Peter Krempa To: libvir-list@redhat.com Date: Fri, 30 Mar 2018 12:59:08 +0200 Message-Id: <70906b7d3517d50341a3ce63c852bb43b77902ac.1522407032.git.pkrempa@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-loop: libvir-list@redhat.com Cc: Peter Krempa Subject: [libvirt] [PATCH 1/9] util: json: Fix freeing of objects appended to virJSONValue X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 30 Mar 2018 10:59:29 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" It was not possible to determine whether virJSONValueObjectAddVArgs and the functions using it would consume a virJSONValue or not when used with the 'a' or 'A' modifier depending on when the loop failed. Fix this by passing in a pointer to the pointer so that it can be cleared once it's successfully consumed and the callers don't have to second-guess leaving a chance of leaking or double freeing the value depending on the ordering. Fix all callers to pass a double pointer too. Signed-off-by: Peter Krempa --- src/qemu/qemu_agent.c | 7 ++----- src/qemu/qemu_block.c | 22 ++++++---------------- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_monitor_json.c | 36 ++++++++++-------------------------- src/util/virjson.c | 10 +++++++--- tests/qemublocktest.c | 4 +--- 6 files changed, 27 insertions(+), 54 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 89183c3f76..b99d5b10e3 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1336,14 +1336,13 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char = **mountpoints, return -1; cmd =3D qemuAgentMakeCommand("guest-fsfreeze-freeze-list", - "a:mountpoints", arg, NULL); + "a:mountpoints", &arg, NULL); } else { cmd =3D qemuAgentMakeCommand("guest-fsfreeze-freeze", NULL); } if (!cmd) goto cleanup; - arg =3D NULL; if (qemuAgentCommand(mon, cmd, &reply, true, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) @@ -1611,12 +1610,10 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr mon, } if (!(cmd =3D qemuAgentMakeCommand("guest-set-vcpus", - "a:vcpus", cpus, + "a:vcpus", &cpus, NULL))) goto cleanup; - cpus =3D NULL; - if (qemuAgentCommand(mon, cmd, &reply, true, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) goto cleanup; diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6f81e9d96c..c0cf6a95ad 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -674,11 +674,9 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSource= Ptr src) "s:driver", "gluster", "s:volume", src->volume, "s:path", src->path, - "a:server", servers, NULL) < 0) + "a:server", &servers, NULL) < 0) goto cleanup; - servers =3D NULL; - if (src->debug && virJSONValueObjectAdd(props, "u:debug", src->debugLevel, NULL) < 0) goto cleanup; @@ -719,7 +717,7 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr = src) "s:driver", protocol, "S:tls-creds", src->tlsAlias, "s:vdisk-id", src->path, - "a:server", server, NULL) < 0) + "a:server", &server, NULL) < 0) virJSONValueFree(server); return ret; @@ -867,14 +865,12 @@ qemuBlockStorageSourceGetNBDProps(virStorageSourcePtr= src) if (virJSONValueObjectCreate(&ret, "s:driver", "nbd", - "a:server", serverprops, + "a:server", &serverprops, "S:export", src->path, "S:tls-creds", src->tlsAlias, NULL) < 0) goto cleanup; - serverprops =3D NULL; - cleanup: virJSONValueFree(serverprops); return ret; @@ -902,13 +898,11 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr= src) "s:image", src->path, "S:snapshot", src->snapshot, "S:conf", src->configFile, - "A:server", servers, + "A:server", &servers, "S:user", username, NULL) < 0) goto cleanup; - servers =3D NULL; - cleanup: virJSONValueFree(servers); return ret; @@ -935,13 +929,11 @@ qemuBlockStorageSourceGetSheepdogProps(virStorageSour= cePtr src) /* libvirt does not support the 'snap-id' and 'tag' properties */ if (virJSONValueObjectCreate(&ret, "s:driver", "sheepdog", - "a:server", serverprops, + "a:server", &serverprops, "s:vdi", src->path, NULL) < 0) goto cleanup; - serverprops =3D NULL; - cleanup: virJSONValueFree(serverprops); return ret; @@ -971,13 +963,11 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr= src) if (virJSONValueObjectCreate(&ret, "s:driver", "ssh", "s:path", src->path, - "a:server", serverprops, + "a:server", &serverprops, "S:user", username, NULL) < 0) goto cleanup; - serverprops =3D NULL; - cleanup: virJSONValueFree(serverprops); return ret; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89fd08b642..3d4130d3c0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1505,7 +1505,7 @@ qemuDiskSourceGetProps(virStorageSourcePtr src) if (!(props =3D qemuBlockStorageSourceGetBackendProps(src))) return NULL; - if (virJSONValueObjectCreate(&ret, "a:file", props, NULL) < 0) { + if (virJSONValueObjectCreate(&ret, "a:file", &props, NULL) < 0) { virJSONValueFree(props); return NULL; } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d80c4f18d1..f38d04f663 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3950,14 +3950,11 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon, cmd =3D qemuMonitorJSONMakeCommand("object-add", "s:qom-type", type, "s:id", objalias, - "A:props", props, + "A:props", &props, NULL); if (!cmd) goto cleanup; - /* @props is part of @cmd now. Avoid double free */ - props =3D NULL; - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; @@ -4133,12 +4130,13 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJ= SONValuePtr actions) int ret =3D -1; virJSONValuePtr cmd; virJSONValuePtr reply =3D NULL; + virJSONValuePtr act =3D actions; bool protect =3D actions->protect; /* We do NOT want to free actions when recursively freeing cmd. */ actions->protect =3D true; cmd =3D qemuMonitorJSONMakeCommand("transaction", - "a:actions", actions, + "a:actions", &act, NULL); if (!cmd) goto cleanup; @@ -4425,15 +4423,12 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, } cmd =3D qemuMonitorJSONMakeCommand("send-key", - "a:keys", keys, + "a:keys", &keys, "p:hold-time", holdtime, NULL); if (!cmd) goto cleanup; - /* @keys is part of @cmd now. Avoid double free */ - keys =3D NULL; - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; @@ -5390,15 +5385,10 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr = mon, if (!(cmd =3D qemuMonitorJSONMakeCommand("query-cpu-model-expansion", "s:type", typeStr, - "a:model", model, + "a:model", &model, NULL))) goto cleanup; - /* model will be freed when cmd is freed. we set model - * to NULL to avoid double freeing. - */ - model =3D NULL; - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; @@ -6263,13 +6253,11 @@ qemuMonitorJSONSetMigrationCapability(qemuMonitorPt= r mon, cap =3D NULL; cmd =3D qemuMonitorJSONMakeCommand("migrate-set-capabilities", - "a:capabilities", caps, + "a:capabilities", &caps, NULL); if (!cmd) goto cleanup; - caps =3D NULL; - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; @@ -6410,7 +6398,7 @@ qemuMonitorJSONBuildInetSocketAddress(const char *hos= t, return NULL; if (virJSONValueObjectCreate(&addr, "s:type", "inet", - "a:data", data, NULL) < 0) { + "a:data", &data, NULL) < 0) { virJSONValueFree(data); return NULL; } @@ -6428,7 +6416,7 @@ qemuMonitorJSONBuildUnixSocketAddress(const char *pat= h) return NULL; if (virJSONValueObjectCreate(&addr, "s:type", "unix", - "a:data", data, NULL) < 0) { + "a:data", &data, NULL) < 0) { virJSONValueFree(data); return NULL; } @@ -6454,13 +6442,10 @@ qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, return ret; if (!(cmd =3D qemuMonitorJSONMakeCommand("nbd-server-start", - "a:addr", addr, + "a:addr", &addr, NULL))) goto cleanup; - /* From now on, @addr is part of @cmd */ - addr =3D NULL; - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; @@ -6763,10 +6748,9 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrI= D, if (!(ret =3D qemuMonitorJSONMakeCommand("chardev-add", "s:id", chrID, - "a:backend", backend, + "a:backend", &backend, NULL))) goto cleanup; - backend =3D NULL; cleanup: VIR_FREE(tlsalias); diff --git a/src/util/virjson.c b/src/util/virjson.c index 6a02ddf0cc..212c158da0 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -109,8 +109,11 @@ virJSONValueGetType(const virJSONValue *value) * d: double precision floating point number * n: json null value * + * The following two cases take a pointer to a pointer to a virJSONValuePt= r. The + * pointer is cleared when the virJSONValuePtr is stolen into the object. * a: json object, must be non-NULL * A: json object, omitted if NULL + * * m: a bitmap represented as a JSON array, must be non-NULL * M: a bitmap represented as a JSON array, omitted if NULL * @@ -241,9 +244,9 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj, case 'A': case 'a': { - virJSONValuePtr val =3D va_arg(args, virJSONValuePtr); + virJSONValuePtr *val =3D va_arg(args, virJSONValuePtr *); - if (!val) { + if (!(*val)) { if (type =3D=3D 'A') continue; @@ -253,7 +256,8 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj, goto cleanup; } - rc =3D virJSONValueObjectAppend(obj, key, val); + if ((rc =3D virJSONValueObjectAppend(obj, key, *val)) =3D=3D 0) + *val =3D NULL; } break; case 'M': diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index e8fac100dd..99584c759c 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -67,11 +67,9 @@ testBackingXMLjsonXML(const void *args) goto cleanup; } - if (virJSONValueObjectCreate(&wrapper, "a:file", backendprops, NULL) <= 0) + if (virJSONValueObjectCreate(&wrapper, "a:file", &backendprops, NULL) = < 0) goto cleanup; - backendprops =3D NULL; - if (!(propsstr =3D virJSONValueToString(wrapper, false))) goto cleanup; --=20 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list