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@xxxxxxxxxx>
---
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 = 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;
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(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;
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 = 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 = 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);
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 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':
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 = NULL;
-
if (!(propsstr = virJSONValueToString(wrapper, false)))
goto cleanup;
--
2.16.2
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
