> -----Original Message-----
> From: Matt Ochs <[email protected]>
> Sent: 03 September 2026 19:47
> To: [email protected]
> Cc: [email protected]; Peter Maydell <[email protected]>; Eric
> Auger <[email protected]>; Nicolin Chen <[email protected]>;
> Shameer Kolothum Thodi <[email protected]>; qemu-
> [email protected]
> Subject: [PATCH] hw/arm/tegra241-cmdqv: Keep VINTF page0 region alive
>
> With CMDQV enabled, resetting a guest after it enables VINTF invokes
> the VINTF page0 unmap path. The resulting crash is intermittent and
> has been observed on the RCU reclaim thread as:
>
> reboot: Restarting system
> double free or corruption (!prev)
> ...
> #5 address_space_dispatch_free
> #6 flatview_destroy
> #7 call_rcu_thread
>
> FlatViews retain raw MemoryRegion pointers and release their references
> asynchronously through RCU. The VINTF page0 unmap path removes the
> subregion and immediately unparents and frees it. An old FlatView can
> then access the freed region during teardown, resulting in a
> use-after-free and heap corruption.
>
> Embed the VINTF page0 MemoryRegion in Tegra241CMDQV and add it only
> once. Use memory_region_set_enabled() as the guest enables and disables
> VINTF. New FlatViews omit the disabled region, while old views continue
> to reference valid storage.
>
> Fixes: 5965b81ce283 ("hw/arm/tegra241-cmdqv: Use mmap'd host VINTF
> page0 for virtual VINTF page0")
> Signed-off-by: Matthew R. Ochs <[email protected]>
> ---
> Reproducer:
>
> Start an Arm virt guest with one passed-through device behind an
> accelerated SMMUv3 configured with cmdqv=on. Add an HMP monitor
> socket:
>
> -monitor unix:/tmp/qmon.sock,server,nowait
>
> The failure can be made reliable without an ASan build by starting QEMU
> with glibc freed-memory poisoning enabled:
>
> GLIBC_TUNABLES=glibc.malloc.tcache_count=0 \
> MALLOC_PERTURB_=165 \
> MALLOC_CHECK_=3 \
> qemu-system-aarch64 <options>
>
> Wait until "info mtree" shows the VINTF page0 region, then reset the
> guest through the monitor:
>
> printf 'system_reset\n' | timeout 5 nc -N -U /tmp/qmon.sock
>
> With the unpatched binary, QEMU crashed on the first reset with SIGSEGV.
> The core showed object_unref() called from address_space_dispatch_free()
> with the object pointer set to 0xa5a5a5a5a5a5a5a5.
>
> Testing:
>
> Unpatched, one CMDQV instance: SIGSEGV on first reset
> Patched, one CMDQV instance: 100/100 resets completed successfully
Thanks for the test results. The patch looks good to me.
One nit below.
> hw/arm/tegra241-cmdqv.c | 22 ++++++++++++----------
> hw/arm/tegra241-cmdqv.h | 3 ++-
> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> index 273633e62937..29123f6267fc 100644
> --- a/hw/arm/tegra241-cmdqv.c
> +++ b/hw/arm/tegra241-cmdqv.c
> @@ -131,35 +131,37 @@ static void
> tegra241_cmdqv_reset_vcmdq_cache(Tegra241CMDQV *cmdqv, int index)
>
> static void tegra241_cmdqv_guest_unmap_vintf_page0(Tegra241CMDQV
> *cmdqv)
> {
> - if (!cmdqv->mr_vintf_page0) {
> + if (!cmdqv->mr_vintf_page0_initialized) {
> return;
> }
>
> - memory_region_del_subregion(&cmdqv->mmio_cmdqv, cmdqv-
> >mr_vintf_page0);
> - object_unparent(OBJECT(cmdqv->mr_vintf_page0));
> - g_free(cmdqv->mr_vintf_page0);
> - cmdqv->mr_vintf_page0 = NULL;
> + /*
> + * Keep the region parented: old FlatViews can retain a pointer to it
> + * until their RCU callbacks have run.
> + */
> + memory_region_set_enabled(&cmdqv->mr_vintf_page0, false);
> }
>
> static void tegra241_cmdqv_guest_map_vintf_page0(Tegra241CMDQV
> *cmdqv)
> {
> char *name;
>
> - if (cmdqv->mr_vintf_page0) {
> + if (cmdqv->mr_vintf_page0_initialized) {
> + memory_region_set_enabled(&cmdqv->mr_vintf_page0, true);
> return;
> }
>
> name = g_strdup_printf("%s vintf-page0",
> memory_region_name(&cmdqv->mmio_cmdqv));
> - cmdqv->mr_vintf_page0 = g_malloc0(sizeof(*cmdqv->mr_vintf_page0));
> - memory_region_init_ram_device_ptr(cmdqv->mr_vintf_page0,
> + memory_region_init_ram_device_ptr(&cmdqv->mr_vintf_page0,
>
> memory_region_owner(&cmdqv->mmio_cmdqv),
> name, VINTF_PAGE_SIZE,
> cmdqv->vintf_page0);
> - memory_region_set_skip_iommu_map(cmdqv->mr_vintf_page0, true);
> + memory_region_set_skip_iommu_map(&cmdqv->mr_vintf_page0, true);
> memory_region_add_subregion_overlap(&cmdqv->mmio_cmdqv,
> CMDQV_VINTF_PAGE0_BASE,
> - cmdqv->mr_vintf_page0, 1);
> + &cmdqv->mr_vintf_page0, 1);
> + cmdqv->mr_vintf_page0_initialized = true;
Do we really need the bool mr_vintf_page0_initialized or can use
memory_region_is_mapped() instead?
Since the region stays a subregion while disabled, memory_region_is_mapped()
still returns true, so it should work for the "initialized" check, I think.
Either way,
Reviewed-by: Shameer Kolothum <[email protected]>
Thanks,
Shameer
