Re: [PATCH v2] kvm: Fix crash due to access uninitialized kvm_state

[David Hildenbrand <david@redhat.com>]

On 31.07.23 14:59, Gavin Shan wrote:
> Runs into core dump on arm64 and the backtrace extracted from the
> core dump is shown as below. It's caused by accessing uninitialized
> @kvm_state in kvm_flush_coalesced_mmio_buffer() due to commit 176d073029
> ("hw/arm/virt: Use machine_memory_devices_init()"), where the machine's
> memory region is added earlier than before.
> 
>      main
>      qemu_init
>      configure_accelerators
>      qemu_opts_foreach
>      do_configure_accelerator
>      accel_init_machine
>      kvm_init
>      virt_kvm_type
>      virt_set_memmap
>      machine_memory_devices_init
>      memory_region_add_subregion
>      memory_region_add_subregion_common
>      memory_region_update_container_subregions
>      memory_region_transaction_begin
>      qemu_flush_coalesced_mmio_buffer
>      kvm_flush_coalesced_mmio_buffer
> 
> Fix it by bailing early in kvm_flush_coalesced_mmio_buffer() on the
> uninitialized @kvm_state. With this applied, no crash is observed on
> arm64.
> 
> Fixes: 176d073029 ("hw/arm/virt: Use machine_memory_devices_init()")
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> v2: Bail early in kvm_flush_coalesced_mmio_buffer() on the uninitialized
>      @kvm_state and improved changelog        (David/Peter)
> ---
>   accel/kvm/kvm-all.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 373d876c05..7b3da8dc3a 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2812,7 +2812,7 @@ void kvm_flush_coalesced_mmio_buffer(void)
>   {
>       KVMState *s = kvm_state;
>   
> -    if (s->coalesced_flush_in_progress) {
> +    if (!s || s->coalesced_flush_in_progress) {
>           return;
>       }
>   

Thanks Gavin!

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb