From b2999ed8ad6d9793b8310a1c7138a0232f637d1d Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Fri, 21 Apr 2023 13:25:50 +0100 Subject: hw/pci-bridge: Fix release ordering by embedding PCIBridgeWindows within PCIBridge The lifetime of the PCIBridgeWindows instance accessed via the windows pointer in struct PCIBridge is managed separately from the PCIBridge itself. Triggered by ./qemu-system-x86_64 -M x-remote -display none -monitor stdio QEMU monitor: device_add cxl-downstream In some error handling paths (such as the above due to attaching a cxl-downstream port anything other than a cxl-upstream port) the g_free() of the PCIBridge windows in pci_bridge_region_cleanup() is called before the final call of flatview_uref() in address_space_set_flatview() ultimately from drain_call_rcu() At one stage this resulted in a crash, currently can still be observed using valgrind which records a use after free. When present, only one instance is allocated. pci_bridge_update_mappings() can operate directly on an instance rather than creating a new one and swapping it in. Thus there appears to be no reason to not directly couple the lifetimes of the two structures by embedding the PCIBridgeWindows within the PCIBridge removing the need for the problematic separate free. Patch is same as was posted deep in the discussion. https://lore.kernel.org/qemu-devel/20230403171232.000020bb@huawei.com/ Reported-by: Thomas Huth Signed-off-by: Jonathan Cameron Message-Id: <20230421122550.28234-1-Jonathan.Cameron@huawei.com> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- include/hw/pci/pci_bridge.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/hw/pci/pci_bridge.h') diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h index 01670e9e65..ea54a81a15 100644 --- a/include/hw/pci/pci_bridge.h +++ b/include/hw/pci/pci_bridge.h @@ -73,7 +73,7 @@ struct PCIBridge { MemoryRegion address_space_mem; MemoryRegion address_space_io; - PCIBridgeWindows *windows; + PCIBridgeWindows windows; pci_map_irq_fn map_irq; const char *bus_name; -- cgit 1.4.1