OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

softmmu/ioport.c: make MemoryRegionPortioList owner of portio_list MemoryRegions

Browse Source 
Currently when portio_list MemoryRegions are freed using portio_list_destroy() the RCU
thread segfaults generating a backtrace similar to that below:

    #0 0x5555599a34b6 in phys_section_destroy ../softmmu/physmem.c:996
    #1 0x5555599a37a3 in phys_sections_free ../softmmu/physmem.c:1011
    #2 0x5555599b24aa in address_space_dispatch_free ../softmmu/physmem.c:2430
    #3 0x55555996a283 in flatview_destroy ../softmmu/memory.c:292
    #4 0x55555a2cb9fb in call_rcu_thread ../util/rcu.c:284
    #5 0x55555a29b71d in qemu_thread_start ../util/qemu-thread-posix.c:541
    #6 0x7ffff4a0cea6 in start_thread nptl/pthread_create.c:477
    #7 0x7ffff492ca2e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfca2e)

The problem here is that portio_list_destroy() unparents the portio_list
MemoryRegions causing them to be freed immediately, however the flatview
still has a reference to the MemoryRegion and so causes a use-after-free
segfault when the RCU thread next updates the flatview.

Solve the lifetime issue by making MemoryRegionPortioList the owner of the
portio_list MemoryRegions, and then reparenting them to the portio_list
owner. This ensures that they can be accessed as QOM children via the
portio_list owner, yet the MemoryRegionPortioList owns the refcount.

Update portio_list_destroy() to unparent the MemoryRegion from the
portio_list owner (while keeping mrpio->mr live until finalization of the
MemoryRegionPortioList), so that the portio_list MemoryRegions remain
allocated until flatview_destroy() removes the final refcount upon the
next flatview update.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230419151652.362717-4-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Mark Cave-Ayland 2023-04-19 16:16:52 +01:00 committed by Paolo Bonzini
parent 28770689c5
commit 690705ca0b
1 changed files with 21 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
softmmu/ioport.c
View File

@ -229,6 +229,8 @@ static void portio_list_add_1(PortioList *piolist,
unsigned off_low, unsigned off_high) unsigned off_low, unsigned off_high)
{ {
MemoryRegionPortioList *mrpio; MemoryRegionPortioList *mrpio;
Object *owner;
char *name;
unsigned i; unsigned i;
/* Copy the sub-list and null-terminate it. */ /* Copy the sub-list and null-terminate it. */
@ -245,8 +247,25 @@ static void portio_list_add_1(PortioList *piolist,
mrpio->ports[i].base = start + off_low; mrpio->ports[i].base = start + off_low;
} }
memory_region_init_io(&mrpio->mr, piolist->owner, &portio_ops, mrpio, /*
* The MemoryRegion owner is the MemoryRegionPortioList since that manages
* the lifecycle via the refcount
*/
memory_region_init_io(&mrpio->mr, OBJECT(mrpio), &portio_ops, mrpio,
piolist->name, off_high - off_low); piolist->name, off_high - off_low);
/* Reparent the MemoryRegion to the piolist owner */
object_ref(&mrpio->mr);
object_unparent(OBJECT(&mrpio->mr));
if (!piolist->owner) {
owner = container_get(qdev_get_machine(), "/unattached");
} else {
owner = piolist->owner;
}
name = g_strdup_printf("%s[*]", piolist->name);
object_property_add_child(owner, name, OBJECT(&mrpio->mr));
g_free(name);
if (piolist->flush_coalesced_mmio) { if (piolist->flush_coalesced_mmio) {
memory_region_set_flush_coalesced(&mrpio->mr); memory_region_set_flush_coalesced(&mrpio->mr);
} }
@ -308,6 +327,7 @@ static void memory_region_portio_list_finalize(Object *obj)
{ {
MemoryRegionPortioList *mrpio = MEMORY_REGION_PORTIO_LIST(obj); MemoryRegionPortioList *mrpio = MEMORY_REGION_PORTIO_LIST(obj);
object_unref(&mrpio->mr);
g_free(mrpio->ports); g_free(mrpio->ports);
} }