[PATCH 1/2] qemu: monitor:Prevent a NULl pointer from being accessed
From: Huang Zijiang <huang.zijiang@zte.com.cn> virJSONValueObjectGetObject maybe return NULL if the key is missing or if value is not the correct TYPE, so we have to prevent a NULl pointer from being accessed. Signed-off-by: Huang Zijiang <huang.zijiang@zte.com.cn> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn> --- src/qemu/qemu_monitor_json.c | 89 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e5164d2..51b40e0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1697,7 +1697,12 @@ qemuMonitorJSONGetStatus(qemuMonitorPtr mon, goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); - + if (!data){ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-status reply was missing return data")); + return -1; + } + if (virJSONValueObjectGetBoolean(data, "running", running) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-status reply was missing running state")); @@ -2018,6 +2023,11 @@ int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-kvm reply was missing return data")); + return -1; + } if (virJSONValueObjectGetBoolean(data, "enabled", &val) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2179,6 +2189,11 @@ qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-balloon reply was missing return data")); + return -1; + } if (virJSONValueObjectGetNumberUlong(data, "actual", &mem) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2280,6 +2295,11 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("reply was missing return data")); + return -1; + } if (!(statsdata = virJSONValueObjectGet(data, "stats"))) { VIR_DEBUG("data does not include 'stats'"); @@ -3478,6 +3498,11 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply, const char *tmp; ret = virJSONValueObjectGetObject(reply, "return"); + if (!ret) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("info migration reply was missing return data")); + return -1; + } if (!(statusstr = virJSONValueObjectGetString(ret, "status"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -3787,6 +3812,11 @@ qemuMonitorJSONQueryDump(qemuMonitorPtr mon, goto cleanup; result = virJSONValueObjectGetObject(reply, "return"); + if (!result) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-dump reply was missing return data")); + return -1; + } ret = qemuMonitorJSONExtractDumpStats(result, stats); @@ -3824,6 +3854,11 @@ qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, goto cleanup; caps = virJSONValueObjectGetObject(reply, "return"); + if (!caps) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-dump-guest-memory-capability reply was missing return data")); + return -1; + } if (!(formats = virJSONValueObjectGetArray(caps, "formats"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -4676,6 +4711,11 @@ qemuMonitorJSONDiskNameLookupOne(virJSONValuePtr image, return NULL; if (top != target) { backing = virJSONValueObjectGetObject(image, "backing-image"); + if (!backing) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("reply was missing return backing-image ")); + return -1; + } return qemuMonitorJSONDiskNameLookupOne(backing, top->backingStore, target); } @@ -5519,6 +5559,12 @@ int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing return data")); + return -1; + } + if (!(qemu = virJSONValueObjectGetObject(data, "qemu"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -5972,6 +6018,11 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, return -1; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply was missing return data")); + return -1; + } if (qemuMonitorJSONParseCPUModelData(data, "query-cpu-model-expansion", fail_no_props, &cpu_model, &cpu_props, @@ -6027,6 +6078,11 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, return -1; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-baseline reply was missing return data")); + return -1; + } if (qemuMonitorJSONParseCPUModelData(data, "query-cpu-model-baseline", false, &cpu_model, &cpu_props, @@ -6067,6 +6123,11 @@ qemuMonitorJSONGetCPUModelComparison(qemuMonitorPtr mon, return -1; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-comparison reply was missing return data")); + return -1; + } if (!(data_result = virJSONValueObjectGetString(data, "result"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -6342,6 +6403,11 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon, goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu-kvm reply was missing return data")); + return -1; + } if (virJSONValueObjectGetBoolean(data, "enabled", enabled) < 0 || virJSONValueObjectGetBoolean(data, "present", present) < 0) { @@ -6823,6 +6889,11 @@ qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon) goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("query-target reply was missing return data")); + return -1; + } if (!(arch = virJSONValueObjectGetString(data, "arch"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -7090,6 +7161,11 @@ qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, goto cleanup; caps = virJSONValueObjectGetObject(reply, "return"); + if (!caps) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("query-sev-capabilities reply was missing return data")); + return -1; + } if (virJSONValueObjectGetNumberUint(caps, "cbitpos", &cbitpos) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -7555,6 +7631,12 @@ qemuMonitorJSONAttachCharDev(qemuMonitorPtr mon, if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { virJSONValuePtr data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("chardev-add reply was missing return data")); + return -1; + } + const char *path; if (!(path = virJSONValueObjectGetString(data, "pty"))) { @@ -9006,6 +9088,11 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitorPtr mon) goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("query-sev-launch-measure reply was missing return data")); + return -1; + } if (!(tmp = virJSONValueObjectGetString(data, "data"))) goto cleanup; -- 1.9.1
On Wed, Feb 12, 2020 at 22:09:42 +0800, Yi Wang wrote:
From: Huang Zijiang <huang.zijiang@zte.com.cn>
virJSONValueObjectGetObject maybe return NULL if the key is missing or if value is not the correct TYPE, so we have to prevent a NULl pointer from being accessed.
If you see below we already check that the function will not return NULL before using it. Is there any bug you are attempting to fix? If yes please also describe the symptoms of it.
Signed-off-by: Huang Zijiang <huang.zijiang@zte.com.cn> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn> --- src/qemu/qemu_monitor_json.c | 89 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e5164d2..51b40e0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1697,7 +1697,12 @@ qemuMonitorJSONGetStatus(qemuMonitorPtr mon, goto cleanup;
data = virJSONValueObjectGetObject(reply, "return"); - + if (!data){ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-status reply was missing return data")); + return -1; + } +
Let's have some more context of this function: if (!(cmd = qemuMonitorJSONMakeCommand("query-status", NULL))) return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) goto cleanup; data = virJSONValueObjectGetObject(reply, "return"); if (virJSONValueObjectGetBoolean(data, "running", running) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", As you can see qemuMonitorJSONCheckReply is used which makes sure that the 'return' object exists in 'reply'. This means that virJSONValueObjectGetObject will return a valid pointer. This was deliberately cleaned up this way some time ago.
if (virJSONValueObjectGetBoolean(data, "running", running) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-status reply was missing running state")); @@ -6027,6 +6078,11 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, return -1;
data = virJSONValueObjectGetObject(reply, "return"); + if (!data) { + (VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-baseline reply was missing return data"));
This won't even compile.
+ return -1; + }
participants (2)
-
Peter Krempa -
Yi Wang