Skip to content

Commit dc85f9f

Browse files
pm215nvmochs
authored andcommitted
hw/pci-host/dino: Don't call pci_register_root_bus() in init
In the dino PCI host bridge device, we call pci_register_root_bus() in the device's instance_init. This is a problem for two reasons * the PCI bridge is then available to the rest of the simulation (e.g. via pci_qdev_find_device()), even though it hasn't yet been realized * we do not attempt to unregister in an instance_deinit, which means that if you go through an instance_init -> deinit lifecycle the freed memory for the host-bridge device is left on the pci_host_bridges list ASAN reports the resulting use-after-free: ==1771223==ERROR: AddressSanitizer: heap-use-after-free on address 0x527000018f80 at pc 0x5b4b9d3369b5 bp 0x7ffd01929980 sp 0x7ffd01929978 WRITE of size 8 at 0x527000018f80 thread T0 #0 0x5b4b9d3369b4 in pci_host_bus_register /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../hw/pci/pci.c:608:5 #1 0x5b4b9d321566 in pci_root_bus_internal_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../hw/pci/pci.c:677:5 NVIDIA#2 0x5b4b9d3215e0 in pci_root_bus_new /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../hw/pci/pci.c:706:5 NVIDIA#3 0x5b4b9d321fe5 in pci_register_root_bus /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../hw/pci/pci.c:751:11 NVIDIA#4 0x5b4b9d390521 in dino_pcihost_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../hw/pci-host/dino.c:473:16 0x527000018f80 is located 1664 bytes inside of 12384-byte region [0x527000018900,0x52700001b960) freed by thread T0 here: #0 0x5b4b9cab185a in free (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/qemu-system-hppa+0x17ad85a) (BuildId: ca496bb2e4fc750ebd289b448bad8d99c0ecd140) #1 0x5b4b9e3ee723 in object_finalize /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../qom/object.c:734:9 NVIDIA#2 0x5b4b9e3e69db in object_unref /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../qom/object.c:1232:9 NVIDIA#3 0x5b4b9ea6173c in qmp_device_list_properties /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../qom/qom-qmp-cmds.c:237:5 NVIDIA#4 0x5b4b9ec4e0f3 in qmp_marshal_device_list_properties /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/qapi/qapi-commands-qdev.c:65:14 previously allocated by thread T0 here: #0 0x5b4b9cab1af3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/qemu-system-hppa+0x17adaf3) (BuildId: ca496bb2e4fc750ebd289b448bad8d99c0ecd140) #1 0x799d8270eb09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75) NVIDIA#2 0x5b4b9e3e75fc in object_new_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../qom/object.c:767:15 NVIDIA#3 0x5b4b9e3e7409 in object_new_with_class /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../qom/object.c:782:12 NVIDIA#4 0x5b4b9ea609a5 in qmp_device_list_properties /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/hppa-asan/../../qom/qom-qmp-cmds.c:206:11 where we allocated one instance of the dino device, put it on the list, freed it, and then trying to allocate a second instance touches the freed memory on the pci_host_bridges list. Fix this by deferring all the setup of memory regions and registering the PCI bridge to the device's realize method. This brings it into line with almost all other PCI host bridges, which call pci_register_root_bus() in realize. Cc: [email protected] Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3118 Fixes: 63901b6 ("dino: move PCI bus initialisation to dino_pcihost_init()") Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Alex Bennée <[email protected]> Tested-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> Message-ID: <[email protected]> (cherry picked from commit e4a1b30) Signed-off-by: Michael Tokarev <[email protected]> (cherry picked from commit 975d8f3) Signed-off-by: Matthew R. Ochs <[email protected]>
1 parent e99268f commit dc85f9f

File tree

1 file changed

+33
-41
lines changed

1 file changed

+33
-41
lines changed

hw/pci-host/dino.c

Lines changed: 33 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -413,43 +413,7 @@ static void dino_pcihost_reset(DeviceState *dev)
413413
static void dino_pcihost_realize(DeviceState *dev, Error **errp)
414414
{
415415
DinoState *s = DINO_PCI_HOST_BRIDGE(dev);
416-
417-
/* Set up PCI view of memory: Bus master address space. */
418-
memory_region_init(&s->bm, OBJECT(s), "bm-dino", 4 * GiB);
419-
memory_region_init_alias(&s->bm_ram_alias, OBJECT(s),
420-
"bm-system", s->memory_as, 0,
421-
0xf0000000 + DINO_MEM_CHUNK_SIZE);
422-
memory_region_init_alias(&s->bm_pci_alias, OBJECT(s),
423-
"bm-pci", &s->pci_mem,
424-
0xf0000000 + DINO_MEM_CHUNK_SIZE,
425-
30 * DINO_MEM_CHUNK_SIZE);
426-
memory_region_init_alias(&s->bm_cpu_alias, OBJECT(s),
427-
"bm-cpu", s->memory_as, 0xfff00000,
428-
0xfffff);
429-
memory_region_add_subregion(&s->bm, 0,
430-
&s->bm_ram_alias);
431-
memory_region_add_subregion(&s->bm,
432-
0xf0000000 + DINO_MEM_CHUNK_SIZE,
433-
&s->bm_pci_alias);
434-
memory_region_add_subregion(&s->bm, 0xfff00000,
435-
&s->bm_cpu_alias);
436-
437-
address_space_init(&s->bm_as, &s->bm, "pci-bm");
438-
}
439-
440-
static void dino_pcihost_unrealize(DeviceState *dev)
441-
{
442-
DinoState *s = DINO_PCI_HOST_BRIDGE(dev);
443-
444-
address_space_destroy(&s->bm_as);
445-
}
446-
447-
static void dino_pcihost_init(Object *obj)
448-
{
449-
DinoState *s = DINO_PCI_HOST_BRIDGE(obj);
450-
PCIHostState *phb = PCI_HOST_BRIDGE(obj);
451-
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
452-
int i;
416+
PCIHostState *phb = PCI_HOST_BRIDGE(dev);
453417

454418
/* Dino PCI access from main memory. */
455419
memory_region_init_io(&s->this_mem, OBJECT(s), &dino_chip_ops,
@@ -476,7 +440,7 @@ static void dino_pcihost_init(Object *obj)
476440
PCI_DEVFN(0, 0), 32, TYPE_PCI_BUS);
477441

478442
/* Set up windows into PCI bus memory. */
479-
for (i = 1; i < 31; i++) {
443+
for (int i = 1; i < 31; i++) {
480444
uint32_t addr = 0xf0000000 + i * DINO_MEM_CHUNK_SIZE;
481445
char *name = g_strdup_printf("PCI Outbound Window %d", i);
482446
memory_region_init_alias(&s->pci_mem_alias[i], OBJECT(s),
@@ -487,9 +451,38 @@ static void dino_pcihost_init(Object *obj)
487451

488452
pci_setup_iommu(phb->bus, &dino_iommu_ops, s);
489453

490-
sysbus_init_mmio(sbd, &s->this_mem);
454+
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->this_mem);
491455

492-
qdev_init_gpio_in(DEVICE(obj), dino_set_irq, DINO_IRQS);
456+
qdev_init_gpio_in(dev, dino_set_irq, DINO_IRQS);
457+
458+
/* Set up PCI view of memory: Bus master address space. */
459+
memory_region_init(&s->bm, OBJECT(s), "bm-dino", 4 * GiB);
460+
memory_region_init_alias(&s->bm_ram_alias, OBJECT(s),
461+
"bm-system", s->memory_as, 0,
462+
0xf0000000 + DINO_MEM_CHUNK_SIZE);
463+
memory_region_init_alias(&s->bm_pci_alias, OBJECT(s),
464+
"bm-pci", &s->pci_mem,
465+
0xf0000000 + DINO_MEM_CHUNK_SIZE,
466+
30 * DINO_MEM_CHUNK_SIZE);
467+
memory_region_init_alias(&s->bm_cpu_alias, OBJECT(s),
468+
"bm-cpu", s->memory_as, 0xfff00000,
469+
0xfffff);
470+
memory_region_add_subregion(&s->bm, 0,
471+
&s->bm_ram_alias);
472+
memory_region_add_subregion(&s->bm,
473+
0xf0000000 + DINO_MEM_CHUNK_SIZE,
474+
&s->bm_pci_alias);
475+
memory_region_add_subregion(&s->bm, 0xfff00000,
476+
&s->bm_cpu_alias);
477+
478+
address_space_init(&s->bm_as, &s->bm, "pci-bm");
479+
}
480+
481+
static void dino_pcihost_unrealize(DeviceState *dev)
482+
{
483+
DinoState *s = DINO_PCI_HOST_BRIDGE(dev);
484+
485+
address_space_destroy(&s->bm_as);
493486
}
494487

495488
static const Property dino_pcihost_properties[] = {
@@ -511,7 +504,6 @@ static void dino_pcihost_class_init(ObjectClass *klass, const void *data)
511504
static const TypeInfo dino_pcihost_info = {
512505
.name = TYPE_DINO_PCI_HOST_BRIDGE,
513506
.parent = TYPE_PCI_HOST_BRIDGE,
514-
.instance_init = dino_pcihost_init,
515507
.instance_size = sizeof(DinoState),
516508
.class_init = dino_pcihost_class_init,
517509
};

0 commit comments

Comments
 (0)