[PATCH] qemu_driver: fix potential null pointer dereference in qemuConnectCPUModelBaseline()
qemuMonitorGetCPUModelExpansion() may return 0 without assigning a value to model_info if QEMU returns a GenericError. Passing NULL to qemuConnectStealCPUModelFromInfo() can lead to the NULL pointer dereference. Check the result before stealing the CPU model to avoid this. Fixes: aa797c6625 (qemu_driver: expand cpu features after baseline) Signed-off-by: Elizaveta Tereshkina <teryoshkina.ea@gmail.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ebf19d7c0f..5922065897 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12382,7 +12382,7 @@ qemuConnectCPUModelBaseline(virQEMUCaps *qemuCaps, false, false, &result) < 0) return NULL; - if (qemuConnectStealCPUModelFromInfo(baseline, &result) < 0) + if (result && qemuConnectStealCPUModelFromInfo(baseline, &result) < 0) return NULL; } -- 2.43.0
On Mon, Oct 05, 2026 at 02:33:00 +0300, Elizaveta Tereshkina wrote:
qemuMonitorGetCPUModelExpansion() may return 0 without assigning a value to model_info if QEMU returns a GenericError. Passing NULL to qemuConnectStealCPUModelFromInfo() can lead to the NULL pointer dereference. Check the result before stealing the CPU model to avoid this.
Can you, please, describe how exactly can qemuMonitorGetCPUModelExpansion return zero without assigning to model_info? Looking at the code the only way the function could return zero is when qemuMonitorJSONParseCPUModel returns zero. And that only happens in one place: *model_info = g_steal_pointer(&machine_model); return 0; machine_model is initialized with g_new0(qemuMonitorCPUModelInfo, 1) just at the beginning of qemuMonitorJSONParseCPUModel. Jirka
Can you, please, describe how exactly can qemuMonitorGetCPUModelExpansion return zero without assigning to model_info?
Looking at the code the only way the function could return zero is when qemuMonitorJSONParseCPUModel returns zero. And that only happens in one place:
*model_info = g_steal_pointer(&machine_model); return 0;
As I understand: qemuMonitorGetCPUModelExpansion() returns qemuMonitorJSONGetCPUModelExpansion() result. In qemuMonitorJSONGetCPUModelExpansion(): ``` if ((rc = qemuMonitorJSONQueryCPUModelExpansionOne(mon, type, &model, &data)) <= 0) return rc; ``` qemuMonitorJSONQueryCPUModelExpansionOne() returns zero if the condition is true: ``` if (qemuMonitorJSONHasError(reply, "GenericError")) return 0; ``` So the qemuMonitorJSONParseCPUModelExpansion with `*model_info = g_steal_pointer` won't even be called because of early return.
On Mon, Oct 12, 2026 at 16:26:23 +0300, Elizaveta wrote:
Can you, please, describe how exactly can qemuMonitorGetCPUModelExpansion return zero without assigning to model_info?
Looking at the code the only way the function could return zero is when qemuMonitorJSONParseCPUModel returns zero. And that only happens in one place:
*model_info = g_steal_pointer(&machine_model); return 0;
As I understand:
qemuMonitorGetCPUModelExpansion() returns qemuMonitorJSONGetCPUModelExpansion() result.
In qemuMonitorJSONGetCPUModelExpansion():
``` if ((rc = qemuMonitorJSONQueryCPUModelExpansionOne(mon, type, &model, &data)) <= 0) return rc; ```
Oh you are right, I missed that line somehow. Jirka
On Mon, Oct 05, 2026 at 02:33:00 +0300, Elizaveta Tereshkina wrote:
qemuMonitorGetCPUModelExpansion() may return 0 without assigning a value to model_info if QEMU returns a GenericError. Passing NULL to qemuConnectStealCPUModelFromInfo() can lead to the NULL pointer dereference. Check the result before stealing the CPU model to avoid this.
Fixes: aa797c6625 (qemu_driver: expand cpu features after baseline) Signed-off-by: Elizaveta Tereshkina <teryoshkina.ea@gmail.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ebf19d7c0f..5922065897 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12382,7 +12382,7 @@ qemuConnectCPUModelBaseline(virQEMUCaps *qemuCaps, false, false, &result) < 0) return NULL;
- if (qemuConnectStealCPUModelFromInfo(baseline, &result) < 0) + if (result && qemuConnectStealCPUModelFromInfo(baseline, &result) < 0) return NULL; }
I was thinking about handling the situation in a better way, such as by reporting an error. But I think this is OK, at this point we already successfully called query-cpu-model-baseline so query-cpu-model-expansion should be implemented as well. They either both exist or none of them. Reviewed-by: Jiri Denemark <jdenemar@redhat.com> and pushed, thanks.
participants (3)
-
Elizaveta -
Elizaveta Tereshkina
-
Jiri Denemark