util: json: Fix freeing of objects appended to virJSONValue

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 <pkrempa@redhat.com>
This commit is contained in:
Peter Krempa 2018-03-30 11:12:57 +02:00
parent 5dda119a44
commit ea520f6b67
6 changed files with 27 additions and 54 deletions

View File

@ -1336,14 +1336,13 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints,
return -1;
cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze-list",
"a:mountpoints", arg, NULL);
"a:mountpoints", &arg, NULL);
} else {
cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze", NULL);
}
if (!cmd)
goto cleanup;
arg = NULL;
if (qemuAgentCommand(mon, cmd, &reply, true,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
@ -1611,12 +1610,10 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr mon,
}
if (!(cmd = qemuAgentMakeCommand("guest-set-vcpus",
"a:vcpus", cpus,
"a:vcpus", &cpus,
NULL)))
goto cleanup;
cpus = NULL;
if (qemuAgentCommand(mon, cmd, &reply, true,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;

View File

@ -674,11 +674,9 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr 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 = 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 = 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 = NULL;
cleanup:
virJSONValueFree(servers);
return ret;
@ -935,13 +929,11 @@ qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr 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 = 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 = NULL;
cleanup:
virJSONValueFree(serverprops);
return ret;

View File

@ -1505,7 +1505,7 @@ qemuDiskSourceGetProps(virStorageSourcePtr src)
if (!(props = qemuBlockStorageSourceGetBackendProps(src)))
return NULL;
if (virJSONValueObjectCreate(&ret, "a:file", props, NULL) < 0) {
if (virJSONValueObjectCreate(&ret, "a:file", &props, NULL) < 0) {
virJSONValueFree(props);
return NULL;
}

View File

@ -3950,14 +3950,11 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon,
cmd = 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 = NULL;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
@ -4133,12 +4130,13 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions)
int ret = -1;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
virJSONValuePtr act = actions;
bool protect = actions->protect;
/* We do NOT want to free actions when recursively freeing cmd. */
actions->protect = true;
cmd = qemuMonitorJSONMakeCommand("transaction",
"a:actions", actions,
"a:actions", &act,
NULL);
if (!cmd)
goto cleanup;
@ -4425,15 +4423,12 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon,
}
cmd = 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 = NULL;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
@ -5390,15 +5385,10 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
if (!(cmd = 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 = NULL;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
@ -6263,13 +6253,11 @@ qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon,
cap = NULL;
cmd = qemuMonitorJSONMakeCommand("migrate-set-capabilities",
"a:capabilities", caps,
"a:capabilities", &caps,
NULL);
if (!cmd)
goto cleanup;
caps = NULL;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
@ -6410,7 +6398,7 @@ qemuMonitorJSONBuildInetSocketAddress(const char *host,
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 *path)
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 = qemuMonitorJSONMakeCommand("nbd-server-start",
"a:addr", addr,
"a:addr", &addr,
NULL)))
goto cleanup;
/* From now on, @addr is part of @cmd */
addr = NULL;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
@ -6763,10 +6748,9 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
if (!(ret = qemuMonitorJSONMakeCommand("chardev-add",
"s:id", chrID,
"a:backend", backend,
"a:backend", &backend,
NULL)))
goto cleanup;
backend = NULL;
cleanup:
VIR_FREE(tlsalias);

View File

@ -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 virJSONValuePtr. 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 = va_arg(args, virJSONValuePtr);
virJSONValuePtr *val = va_arg(args, virJSONValuePtr *);
if (!val) {
if (!(*val)) {
if (type == 'A')
continue;
@ -253,7 +256,8 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
goto cleanup;
}
rc = virJSONValueObjectAppend(obj, key, val);
if ((rc = virJSONValueObjectAppend(obj, key, *val)) == 0)
*val = NULL;
} break;
case 'M':

View File

@ -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 = NULL;
if (!(propsstr = virJSONValueToString(wrapper, false)))
goto cleanup;