[PATCH] hw/hyperv: Avoid crash if hyperv_find_cpu() passed invalid vp_index

[PATCH] hw/hyperv: Avoid crash if hyperv_find_cpu() passed invalid vp_index

[Peter Maydell]

The hyperv_find_cpu() function finds a CPU from a CPU index; this is
basically a wrapper around qemu_get_cpu().  It is allowed to fail, in
which case it returns NULL, which its caller handles.  However, it
includes an assertion check which accidentally assumes the CPU
pointer is non-NULL.

We could assert only if cs != NULL, but the assertion here is not
doing anything interesting -- hyperv_vp_index() is a trivial wrapper
returning cs->cpu_index, so this is effectively asserting that
qemu_get_cpu() did what it claims to do, i.e.  returned us the CPU
matching the index we gave it.  qemu_get_cpu() is a simple "iterate
through list and find matching CPU" which is unlikely to be buggy,
and we don't feel the need to sanity-check it in any of our other
many uses of it.  Drop the assertion entirely.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3568
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Checked only with make check / check-functional...
---
 hw/hyperv/hyperv.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 4d90032785..900ff80213 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -237,9 +237,7 @@ struct HvSintRoute {
 
 static CPUState *hyperv_find_vcpu(uint32_t vp_index)
 {
-    CPUState *cs = qemu_get_cpu(vp_index);
-    assert(hyperv_vp_index(cs) == vp_index);
-    return cs;
+    return qemu_get_cpu(vp_index);
 }
 
 /*
-- 
2.43.0

Re: [PATCH] hw/hyperv: Avoid crash if hyperv_find_cpu() passed invalid vp_index

[Daniel P. Berrangé]

On Tue, Jun 30, 2026 at 09:48:55AM +0100, Peter Maydell wrote:
> The hyperv_find_cpu() function finds a CPU from a CPU index; this is
> basically a wrapper around qemu_get_cpu().  It is allowed to fail, in
> which case it returns NULL, which its caller handles.  However, it
> includes an assertion check which accidentally assumes the CPU
> pointer is non-NULL.
> 
> We could assert only if cs != NULL, but the assertion here is not
> doing anything interesting -- hyperv_vp_index() is a trivial wrapper
> returning cs->cpu_index, so this is effectively asserting that
> qemu_get_cpu() did what it claims to do, i.e.  returned us the CPU
> matching the index we gave it.  qemu_get_cpu() is a simple "iterate
> through list and find matching CPU" which is unlikely to be buggy,
> and we don't feel the need to sanity-check it in any of our other
> many uses of it.  Drop the assertion entirely.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3568
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Checked only with make check / check-functional...
> ---
>  hw/hyperv/hyperv.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH] hw/hyperv: Avoid crash if hyperv_find_cpu() passed invalid vp_index

[Maciej S. Szmigiero]

On 30.06.2026 10:48, Peter Maydell wrote:
> The hyperv_find_cpu() function finds a CPU from a CPU index; this is
> basically a wrapper around qemu_get_cpu().  It is allowed to fail, in
> which case it returns NULL, which its caller handles.  However, it
> includes an assertion check which accidentally assumes the CPU
> pointer is non-NULL.
> 
> We could assert only if cs != NULL, but the assertion here is not
> doing anything interesting -- hyperv_vp_index() is a trivial wrapper
> returning cs->cpu_index, so this is effectively asserting that
> qemu_get_cpu() did what it claims to do, i.e.  returned us the CPU
> matching the index we gave it.  qemu_get_cpu() is a simple "iterate
> through list and find matching CPU" which is unlikely to be buggy,
> and we don't feel the need to sanity-check it in any of our other
> many uses of it.  Drop the assertion entirely.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3568
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Checked only with make check / check-functional...
> ---
>   hw/hyperv/hyperv.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
> index 4d90032785..900ff80213 100644
> --- a/hw/hyperv/hyperv.c
> +++ b/hw/hyperv/hyperv.c
> @@ -237,9 +237,7 @@ struct HvSintRoute {
>   
>   static CPUState *hyperv_find_vcpu(uint32_t vp_index)
>   {
> -    CPUState *cs = qemu_get_cpu(vp_index);
> -    assert(hyperv_vp_index(cs) == vp_index);
> -    return cs;
> +    return qemu_get_cpu(vp_index);
>   }
>   
>   /*

Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>

Thanks,
Maciej

Re: [PATCH] hw/hyperv: Avoid crash if hyperv_find_cpu() passed invalid vp_index

[Philippe Mathieu-Daudé]

On 30/6/26 10:48, Peter Maydell wrote:
> The hyperv_find_cpu() function finds a CPU from a CPU index; this is
> basically a wrapper around qemu_get_cpu().  It is allowed to fail, in
> which case it returns NULL, which its caller handles.  However, it
> includes an assertion check which accidentally assumes the CPU
> pointer is non-NULL.
> 
> We could assert only if cs != NULL, but the assertion here is not
> doing anything interesting -- hyperv_vp_index() is a trivial wrapper
> returning cs->cpu_index, so this is effectively asserting that
> qemu_get_cpu() did what it claims to do, i.e.  returned us the CPU
> matching the index we gave it.  qemu_get_cpu() is a simple "iterate
> through list and find matching CPU" which is unlikely to be buggy,
> and we don't feel the need to sanity-check it in any of our other
> many uses of it.  Drop the assertion entirely.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3568
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Checked only with make check / check-functional...
> ---
>   hw/hyperv/hyperv.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)

Queued via hw-misc tree, thanks.