[PATCH] qemu_driver: fix potential null pointer dereference in qemuConnectCPUModelBaseline()

Elizaveta Tereshkina posted 1 patch 2 weeks, 5 days ago
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] qemu_driver: fix potential null pointer dereference in qemuConnectCPUModelBaseline()
Posted by Elizaveta Tereshkina 2 weeks, 5 days ago
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
Re: [PATCH] qemu_driver: fix potential null pointer dereference in qemuConnectCPUModelBaseline()
Posted by Jiri Denemark via Devel 1 week, 4 days ago
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.
Re: [PATCH] qemu_driver: fix potential null pointer dereference in qemuConnectCPUModelBaseline()
Posted by Jiri Denemark via Devel 1 week, 5 days ago
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
Re: [PATCH] qemu_driver: fix potential null pointer dereference in qemuConnectCPUModelBaseline()
Posted by Elizaveta 1 week, 5 days ago
> 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.
Re: [PATCH] qemu_driver: fix potential null pointer dereference in qemuConnectCPUModelBaseline()
Posted by Jiri Denemark via Devel 1 week, 5 days ago
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