[PATCH] hw/arm/tegra241-cmdqv: Keep VINTF page0 region alive

[Matthew R. Ochs]

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 <mo...@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

 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;
     g_free(name);
 }
 
diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
index de4c1e53358f..502fb1e6ca54 100644
--- a/hw/arm/tegra241-cmdqv.h
+++ b/hw/arm/tegra241-cmdqv.h
@@ -49,7 +49,8 @@ typedef struct Tegra241CMDQV {
     IOMMUFDVeventq *veventq;
     IOMMUFDHWqueue *vcmdq[TEGRA241_CMDQV_MAX_CMDQ];
     void *vintf_page0;
-    MemoryRegion *mr_vintf_page0;
+    MemoryRegion mr_vintf_page0;
+    bool mr_vintf_page0_initialized;
 
     /* CMDQ-V Config page register cache */
     uint32_t config;
-- 
2.50.1