hw/arm/tegra241-cmdqv.c | 21 +++++++++++---------- hw/arm/tegra241-cmdqv.h | 2 +- 2 files changed, 12 insertions(+), 11 deletions(-)
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.
Keeping the region initialized across VINTF disable and reset is safe
because the vIOMMU association and its VINTF page0 mmap remain stable
once the guest CMDQ has been initialized. QEMU blocks hot-unplug of the
device that established the association, so later hot-adds reuse it
instead of associating the initialized guest CMDQ with a different host
SMMUv3. The CMDQV free_viommu path is therefore only invoked while
unwinding initial allocation, before guest CMDQ initialization.
Fixes: 5965b81ce283 ("hw/arm/tegra241-cmdqv: Use mmap'd host VINTF page0 for virtual VINTF page0")
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
Signed-off-by: Matthew R. Ochs <mochs@nvidia.com>
---
v2:
- Use memory_region_is_mapped() instead of an explicit initialization
flag, as suggested by Shameer.
- Link to v1: https://lore.kernel.org/all/20260903184710.2052780-1-mochs@nvidia.com/
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
Patched, four CMDQV instances: 100/100 resets completed successfully
hw/arm/tegra241-cmdqv.c | 21 +++++++++++----------
hw/arm/tegra241-cmdqv.h | 2 +-
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
index 273633e62937..51357885b5c4 100644
--- a/hw/arm/tegra241-cmdqv.c
+++ b/hw/arm/tegra241-cmdqv.c
@@ -131,35 +131,36 @@ 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 (!memory_region_is_mapped(&cmdqv->mr_vintf_page0)) {
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 (memory_region_is_mapped(&cmdqv->mr_vintf_page0)) {
+ 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);
g_free(name);
}
diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
index de4c1e53358f..bba985099e3b 100644
--- a/hw/arm/tegra241-cmdqv.h
+++ b/hw/arm/tegra241-cmdqv.h
@@ -49,7 +49,7 @@ typedef struct Tegra241CMDQV {
IOMMUFDVeventq *veventq;
IOMMUFDHWqueue *vcmdq[TEGRA241_CMDQV_MAX_CMDQ];
void *vintf_page0;
- MemoryRegion *mr_vintf_page0;
+ MemoryRegion mr_vintf_page0;
/* CMDQ-V Config page register cache */
uint32_t config;
--
2.50.1On Fri, 11 Sept 2026 at 20:04, Matthew R. Ochs <mochs@nvidia.com> wrote:
>
> 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.
>
> Keeping the region initialized across VINTF disable and reset is safe
> because the vIOMMU association and its VINTF page0 mmap remain stable
> once the guest CMDQ has been initialized. QEMU blocks hot-unplug of the
> device that established the association, so later hot-adds reuse it
> instead of associating the initialized guest CMDQ with a different host
> SMMUv3. The CMDQV free_viommu path is therefore only invoked while
> unwinding initial allocation, before guest CMDQ initialization.
>
> Fixes: 5965b81ce283 ("hw/arm/tegra241-cmdqv: Use mmap'd host VINTF page0 for virtual VINTF page0")
> Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
> Signed-off-by: Matthew R. Ochs <mochs@nvidia.com>
> ---
> v2:
> - Use memory_region_is_mapped() instead of an explicit initialization
> flag, as suggested by Shameer.
> - Link to v1: https://lore.kernel.org/all/20260903184710.2052780-1-mochs@nvidia.com/
>
> 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
> Patched, four CMDQV instances: 100/100 resets completed successfully
>
> hw/arm/tegra241-cmdqv.c | 21 +++++++++++----------
> hw/arm/tegra241-cmdqv.h | 2 +-
> 2 files changed, 12 insertions(+), 11 deletions(-)
> static void tegra241_cmdqv_guest_map_vintf_page0(Tegra241CMDQV *cmdqv)
> {
> char *name;
>
> - if (cmdqv->mr_vintf_page0) {
> + if (memory_region_is_mapped(&cmdqv->mr_vintf_page0)) {
Here we call memory_region_mapped() on mr_vintf_page0...
> + 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);
...but we don't actually initialize the MemoryRegion until here.
I don't think you should assume that memory_region_* functions
will do anything sensible on a zeroed-out lump of memory.
thanks
-- PMM
> On Sep 14, 2026, at 07:29, Peter Maydell <peter.maydell@linaro.org> wrote:
> On Fri, 11 Sept 2026 at 20:04, Matthew R. Ochs <mochs@nvidia.com> wrote:
>>
>>
>> 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);
>
> ...but we don't actually initialize the MemoryRegion until here.
>
> I don't think you should assume that memory_region_* functions
> will do anything sensible on a zeroed-out lump of memory.
You’re right. Although the current implementation of
memory_region_is_mapped() only examines fields that are zero before
initialization, using it here relies on the internal representation of
an uninitialized MemoryRegion.
memory_region_init_ram_device_ptr() has not yet called
object_initialize() on the embedded region at that point. I’ll restore
the explicit initialization flag from v1 in v3 so that no MemoryRegion
API is called before the region has been initialized.
Thanks for catching this.
-matt
> -----Original Message-----
> From: Matt Ochs <mochs@nvidia.com>
> Sent: 14 September 2026 16:56
> To: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; Eric Auger
> <eric.auger@redhat.com>; Nicolin Chen <nicolinc@nvidia.com>; Shameer
> Kolothum Thodi <skolothumtho@nvidia.com>; qemu-stable@nongnu.org
> Subject: Re: [PATCH v2] hw/arm/tegra241-cmdqv: Keep VINTF page0 region
> alive
>
>
> > On Sep 14, 2026, at 07:29, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> > On Fri, 11 Sept 2026 at 20:04, Matthew R. Ochs <mochs@nvidia.com>
> wrote:
> >>
> >>
> >> 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);
> >
> > ...but we don't actually initialize the MemoryRegion until here.
> >
> > I don't think you should assume that memory_region_* functions
> > will do anything sensible on a zeroed-out lump of memory.
>
> You're right. Although the current implementation of
> memory_region_is_mapped() only examines fields that are zero before
> initialization, using it here relies on the internal representation of
> an uninitialized MemoryRegion.
>
> memory_region_init_ram_device_ptr() has not yet called
> object_initialize() on the embedded region at that point. I'll restore
> the explicit initialization flag from v1 in v3 so that no MemoryRegion
> API is called before the region has been initialized.
Or we could avoid the check altogether by tying the MemoryRegion lifetime
to the vintf_page0 mmap.
- Move the initialization of mr_vintf_page0 to
tegra241_cmdqv_alloc_viommu(), just before the successful return.
Add it as a subregion, but disabled with memory_region_set_enabled(mr, false).
- tegra241_cmdqv_guest_map_vintf_page0() and
tegra241_cmdqv_guest_unmap_vintf_page0() then become just
memory_region_set_enabled(&cmdqv->mr_vintf_page0, true/false).
- In tegra241_cmdqv_free_viommu(), remove the subregion and unparent
the region before the munmap.
This way we tie the MemoryRegion lifetime to the vintf_page0
mmap/munmap, and on VINTF0_CONFIG.ENABLE set/clear we just toggle
memory_region_set_enabled().
Thoughts?
Thanks,
Shameer
> On Sep 14, 2026, at 11:14, Shameer Kolothum Thodi <skolothumtho@nvidia.com> wrote:
>> -----Original Message-----
>> From: Matt Ochs <mochs@nvidia.com>
>> Sent: 14 September 2026 16:56
>> To: Peter Maydell <peter.maydell@linaro.org>
>> Cc: qemu-devel@nongnu.org; qemu-arm@nongnu.org; Eric Auger
>> <eric.auger@redhat.com>; Nicolin Chen <nicolinc@nvidia.com>; Shameer
>> Kolothum Thodi <skolothumtho@nvidia.com>; qemu-stable@nongnu.org
>> Subject: Re: [PATCH v2] hw/arm/tegra241-cmdqv: Keep VINTF page0 region
>> alive
>>
>>
>>> On Sep 14, 2026, at 07:29, Peter Maydell <peter.maydell@linaro.org>
>> wrote:
>>> On Fri, 11 Sept 2026 at 20:04, Matthew R. Ochs <mochs@nvidia.com>
>> wrote:
>>>>
>>>>
>>>> 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);
>>>
>>> ...but we don't actually initialize the MemoryRegion until here.
>>>
>>> I don't think you should assume that memory_region_* functions
>>> will do anything sensible on a zeroed-out lump of memory.
>>
>> You're right. Although the current implementation of
>> memory_region_is_mapped() only examines fields that are zero before
>> initialization, using it here relies on the internal representation of
>> an uninitialized MemoryRegion.
>>
>> memory_region_init_ram_device_ptr() has not yet called
>> object_initialize() on the embedded region at that point. I'll restore
>> the explicit initialization flag from v1 in v3 so that no MemoryRegion
>> API is called before the region has been initialized.
>
> Or we could avoid the check altogether by tying the MemoryRegion lifetime
> to the vintf_page0 mmap.
>
> - Move the initialization of mr_vintf_page0 to
> tegra241_cmdqv_alloc_viommu(), just before the successful return.
> Add it as a subregion, but disabled with memory_region_set_enabled(mr, false).
>
> - tegra241_cmdqv_guest_map_vintf_page0() and
> tegra241_cmdqv_guest_unmap_vintf_page0() then become just
> memory_region_set_enabled(&cmdqv->mr_vintf_page0, true/false).
>
> - In tegra241_cmdqv_free_viommu(), remove the subregion and unparent
> the region before the munmap.
>
> This way we tie the MemoryRegion lifetime to the vintf_page0
> mmap/munmap, and on VINTF0_CONFIG.ENABLE set/clear we just toggle
> memory_region_set_enabled().
Thanks, this approach makes sense to me and addresses Peter's concern
without requiring a separate initialization flag.
I have reworked the patch as suggested. The VINTF page0 MemoryRegion is
initialized and added disabled after successful CMDQV vIOMMU allocation.
Guest enable, disable, and reset now only toggle the region's enabled
state. The allocation-unwind path removes and unparents the still-disabled
region before releasing its mmap.
I will send this implementation as v3 shortly.
-matt
© 2016 - 2026 Red Hat, Inc.