(IGNORE THIS ONE) see the other one in the main repo#1
Draft
lexbaileylowrisc wants to merge 12 commits into
Draft
(IGNORE THIS ONE) see the other one in the main repo#1lexbaileylowrisc wants to merge 12 commits into
lexbaileylowrisc wants to merge 12 commits into
Conversation
This is the start of efforts to contribute code from the lowRISC fork of QEMU to bring the QEMU support for OpenTitan machines in line with Earl Grey 1.0 by adding a new machine called ot-earlgrey. This work was done by Emmanuel Blot and Loïc Lefort from Rivos, and various people at lowRISC. This patch set in particular adds the basic machine definition, and one of the peripherals required for it: the alert handler device. I previously emailed asking for advice on how to test our changes in a way that is acceptable upstream, and how best to submit such a large set of changes, if anyone has more thoughts on that, please do reply to that email. (https://lists.nongnu.org/archive/html/qemu-riscv/2026-02/msg00139.html) There's lots more commits to come, I don't want to dump too much on the mailing list all at once. For now this is a small first step. This patch set should give a flavour of the style of what's to come, and I'm expecting there will be some changes required. We'd just like to understand what will be needed to make this acceptable as soon as we can before we try to upstream lots more of these changes. In particular, most of our new peripheral blocks are currently in the directory called hw/opentitan. In our fork we have about 60 blocks in this directory, we have put them there to keep them organised as opentitan-specific blocks. Is this a suitable place for them? would you prefer a different structure? Thanks! To: qemu-devel@nongnu.org Cc: Palmer Dabbelt <palmer@dabbelt.com> Cc: Alistair Francis <alistair.francis@wdc.com> Cc: Weiwei Li <liwei1518@gmail.com> Cc: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Cc: Liu Zhiwei <zhiwei_liu@linux.alibaba.com> Cc: qemu-riscv@nongnu.org Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Cc: Alistair Francis <alistair@alistair23.me> Cc: Pierrick Bouvier <pierrick.bouvier@linaro.org> Cc: Dr. David Alan Gilbert <dave@treblig.org> Cc: Daniel P. Berrangé <berrange@redhat.com> Cc: Philippe Mathieu-Daudé <philmd@linaro.org> Cc: lowRISC <qemu-maintainers@lowrisc.org> Cc: nabihestefan@google.com <nabihestefan@google.com> Cc: Amit Kumar-Hermosillo <amitkh@google.com> Signed-off-by: Lex Bailey <lex.bailey@lowrisc.org> --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 1, "change-id": "20260217-opentitan-earlgrey-474a14215feb", "prefixes": [ "RFC" ] } }
Follow vendor-device syntax used with other RISCV cores Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
Use a separate Kconfig symbols for Ibex UART, Timer, and SPI devices: having an Ibex CPU does not imply usage of these specific implementations. Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Feb 19, 2026
…tls tests
If you run the TLS tests under a clang undefined-behaviour sanitizer build
it will fall over like this:
../../tests/unit/crypto-tls-psk-helpers.c:53:12: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/unistd.h:858:48: note: nonnull attribute specified here
#0 0x62bd810762ee in test_tls_psk_cleanup /home/pm215/qemu/build/clang/../../tests/unit/crypto-tls-psk-helpers.c:53:5
#1 0x62bd81073f89 in migrate_hook_end_tls_psk /home/pm215/qemu/build/clang/../../tests/qtest/migration/tls-tests.c:101:5
#2 0x62bd81062ef0 in test_precopy_common /home/pm215/qemu/build/clang/../../tests/qtest/migration/framework.c:947:9
This happens because test_precopy_tcp_no_tls() uses a custom start_hook
that only sets a couple of parameters, but reuses the tsk_psk end_hook.
However, the end_hook runs cleanup that assumes that the data was set
up by migrate_hook_start_tls_psk_common(). In particular, it will
unconditionally call test_tls_psk_cleanup(data->pskfile), and
test_tls_psk_cleanup() will unconditionally unlink() the filename it
is passed, which is undefined behaviour if you pass it a NULL pointer.
Instead of creating a TestMigrateTLSPSKData struct which we never set
any fields in and requiring the migrate_hook_end_tls_psk() hook to
cope with that, don't allocate the struct in the start_hook. Then
there is nothing we need to clean up, and we can set the end_hook
to NULL (which the test framework will interpret as "don't call
any end_hook").
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260212114747.1103466-1-peter.maydell@linaro.org
[no need to copy stable]
Signed-off-by: Fabiano Rosas <farosas@suse.de>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Feb 19, 2026
Problem: Two issues with COM initialization: Issue #1: Incorrect call order - requester_init() called CoInitializeSecurity first - Per Microsoft documentation, CoInitialize() must be called BEFORE CoInitializeSecurity() Issue #2: Incorrect call location - CoInitializeSecurity was called from dll instead of the main process - Per Microsoft documentation, CoInitializeSecurity() must be called exactly once per process from the main executable, not from a DLL Reference: https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-coinitializesecurity https://learn.microsoft.com/en-us/windows/win32/api/objbase/nf-objbase-coinitialize This caused incorrect COM initialization, preventing VSS Writers from calling back via IVssWriterCallback (hr = 0x80070005, Access denied, Event ID 8194). Fix: - Initialize COM in main.c for both service and CLI modes - Call CoInitialize() followed by CoInitializeSecurity() in correct order in the main thread before any VSS operations - Add proper CoUninitialize() cleanup Result: VSS Writers can now successfully call back to the agent. Event ID 8194 error is resolved. Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com> Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com> Tested-by: Dehan Meng <demeng@redhat.com> Link: https://lore.kernel.org/qemu-devel/20260211101744.757548-1-eashurov@redhat.com Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
c6c9406 to
b25d9af
Compare
0ee3636 to
99617b6
Compare
Signed-off-by: Lex Bailey <lex.bailey@lowrisc.org>
Signed-off-by: Emmanuel Blot <eblot@rivosinc.com> Includes existing MIT licenced code (already published elsewhere)
Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
1. Define a generic version of lowrisc-ibex core that can be used in several machines: - leave MISA empty so that generic properties can be defined for this core - remove all arbitrary default properties but ISA I,C,U which are mandatory for ibex - define default mtvec which is only support vectored mode - update privilege version (1.12) according to the Ibex documentation - define ibex architecture identifier - remove hart array (mostly useless, its definition is incoherent and prevent from applying properties to CPU cores) 2. Add an EarlGrey machine that uses this new definition Signed-off-by: Loïc Lefort <loic@rivosinc.com> Signed-off-by: Emmanuel Blot <eblot@rivosinc.com> Signed-off-by: Lex Bailey <lex.bailey@lowrisc.org>
Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
also adds property no_epmp_cfg on EarlGrey machine to disable default ePMP configuration. Usage: qemu-system-riscv32 -M ot-earlgrey,no-epmp-cfg=true [...] Signed-off-by: Emmanuel Blot <eblot@rivosinc.com> Signed-off-by: Loïc Lefort <loic@rivosinc.com>
Signed-off-by: Loïc Lefort <loic@rivosinc.com>
Signed-off-by: Emmanuel Blot <eblot@rivosinc.com> Includes existing MIT licenced code (already published elsewhere)
Signed-off-by: Lex Bailey <lex.bailey@lowrisc.org> Includes existing MIT licenced code (already published elsewhere)
99617b6 to
9ed7dac
Compare
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
Commit e27194e ("virtio-gpu-virgl: correct parent for blob memory region") made the name member of MemoryRegion unset, causing a NULL pointer dereference[1]: > Thread 2 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. > (gdb) bt > #0 0x00007ffff56565e2 in __strcmp_evex () at /lib64/libc.so.6 > #1 0x0000555555841bdb in find_fd (head=0x5555572337d0 <cpr_state>, > name=0x0, id=0) at ../migration/cpr.c:68 > #2 cpr_delete_fd (name=name@entry=0x0, id=id@entry=0) at > ../migration/cpr.c:77 > lowRISC#3 0x000055555582290a in qemu_ram_free (block=0x7ff7e93aa7f0) at > ../system/physmem.c:2615 > lowRISC#4 0x000055555581ae02 in memory_region_finalize (obj=<optimized out>) > at ../system/memory.c:1816 > lowRISC#5 0x0000555555a70ab9 in object_deinit (obj=<optimized out>, > type=<optimized out>) at ../qom/object.c:715 > lowRISC#6 object_finalize (data=0x7ff7e936eff0) at ../qom/object.c:729 > lowRISC#7 object_unref (objptr=0x7ff7e936eff0) at ../qom/object.c:1232 > lowRISC#8 0x0000555555814fae in memory_region_unref (mr=<optimized out>) at > ../system/memory.c:1848 > lowRISC#9 flatview_destroy (view=0x555559ed6c40) at ../system/memory.c:301 > lowRISC#10 0x0000555555bfc122 in call_rcu_thread (opaque=<optimized out>) at > ../util/rcu.c:324 > lowRISC#11 0x0000555555bf17a7 in qemu_thread_start (args=0x555557b99520) at > ../util/qemu-thread-posix.c:393 > lowRISC#12 0x00007ffff556f464 in start_thread () at /lib64/libc.so.6 > lowRISC#13 0x00007ffff55f25ac in __clone3 () at /lib64/libc.so.6 The intention of the aforementioned commit is to prevent a MemoryRegion from parenting itself while its references is counted indendependently of the device. To achieve the same goal, add a type of QOM objects that count references and parent MemoryRegions. [1] https://lore.kernel.org/qemu-devel/4eb93d7a-1fa9-4b3c-8ad7-a2eb64f025a0@collabora.com/ Cc: qemu-stable@nongnu.org Fixes: e27194e ("virtio-gpu-virgl: correct parent for blob memory region") Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> Tested-by: Joelle van Dyne <j@getutm.app> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <20260214-region-v1-1-229f00ae1f38@rsg.ci.i.u-tokyo.ac.jp>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
When removing the 'emergency-write' property in commit d0660e5 we neglected to remove the code reducing the virtio_console_config structure size, allowing to access up to the unallocated 'emerg_wr' field. Can be reproduced running: $ cat << EOF | qemu-system-i386 -nodefaults \ -machine q35 -m 512M \ -device virtio-serial \ -display none \ -machine accel=qtest -qtest stdio outl 0xcf8 0x80000810 outl 0xcfc 0xc000 outl 0xcf8 0x80000804 outw 0xcfc 0x01 outl 0xc014 0x00 EOF ==3210206==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x502000090858 at pc 0x5638f1300a9b bp 0x7fff6b525b80 sp 0x7fff6b525b70 READ of size 4 at 0x502000090858 thread T0 #0 0x5638f1300a9a in set_config hw/char/virtio-serial-bus.c:590 #1 0x5638f0bccdcf in virtio_config_writel hw/virtio/virtio-config-io.c:104 #2 0x5638f0bd0c89 in virtio_pci_config_write hw/virtio/virtio-pci.c:637 lowRISC#3 0x5638f0cf90cf in memory_region_write_accessor system/memory.c:491 lowRISC#4 0x5638f0cf975b in access_with_adjusted_size system/memory.c:567 lowRISC#5 0x5638f0d01d3f in memory_region_dispatch_write system/memory.c:1547 lowRISC#6 0x5638f0d2fa1e in address_space_stm_internal system/memory_ldst.c.inc:85 lowRISC#7 0x5638f0d30013 in address_space_stl_le system/memory_ldst_endian.c.inc:53 lowRISC#8 0x5638f0ceb568 in cpu_outl system/ioport.c:79 lowRISC#9 0x5638f0d3c0f9 in qtest_process_command system/qtest.c:483 0x502000090858 is located 0 bytes to the right of 8-byte region [0x502000090850,0x502000090858) allocated by thread T0 here: #0 0x7f0dc32cba57 in __interceptor_calloc src/libsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x7f0dc2382c50 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5ec50) #2 0x5638f1303c27 in virtio_serial_device_realize hw/char/virtio-serial-bus.c:1046 lowRISC#3 0x5638f1396a9c in virtio_device_realize hw/virtio/virtio.c:4053 lowRISC#4 0x5638f13ea370 in device_set_realized hw/core/qdev.c:523 lowRISC#5 0x5638f13fdaf6 in property_set_bool qom/object.c:2376 lowRISC#6 0x5638f13f9098 in object_property_set qom/object.c:1450 lowRISC#7 0x5638f140283c in object_property_set_qobject qom/qom-qobject.c:28 lowRISC#8 0x5638f13f9616 in object_property_set_bool qom/object.c:1520 lowRISC#9 0x5638f13e91cc in qdev_realize hw/core/qdev.c:276 lowRISC#10 0x5638f0c3d94b in virtio_serial_pci_realize hw/virtio/virtio-serial-pci.c:69 lowRISC#11 0x5638f0bda886 in virtio_pci_realize hw/virtio/virtio-pci.c:2351 lowRISC#12 0x5638f09bc2ae in pci_qdev_realize hw/pci/pci.c:2310 lowRISC#13 0x5638f0bdb2f2 in virtio_pci_dc_realize hw/virtio/virtio-pci.c:2473 lowRISC#14 0x5638f13ea370 in device_set_realized hw/core/qdev.c:523 SUMMARY: AddressSanitizer: heap-buffer-overflow hw/char/virtio-serial-bus.c:590 in set_config Fixes: d0660e5 ("hw/char/virtio-serial: Do not expose the 'emergency-write' property") Reported-by: Alexander Bulekov <alxndr@bu.edu> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3303 Buglink: https://issues.oss-fuzz.com/issues/484647006 Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Tested-by: Alexander Bulekov <alxndr@bu.edu> Message-Id: <20260216205527.45938-1-philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The eth_header is not actually guaranteed to be aligned. We attempt
to deal with this in some places such as net_checksum_calculate() by
using lduw_be_p() and so on to access the fields, but this is not
sufficient to be correct, because even accessing a byte member within
a misaligned struct is undefined behaviour. The clang sanitizer will
emit an error like this if you run the sifive_u_mmc functional test
with sanitizers enabled:
../../net/checksum.c:78:47: runtime error: member access within misaligned address 0x561f52f35011 for type 'struct eth_header', which requires 2 byte alignment
0x561f52f35011: note: pointer points here
00 00 00 00 33 33 00 00 00 16 52 54 00 12 34 56 86 dd 60 00 00 00 00 24 00 01 00 00 00 00 00 00
^
#0 0x561f20608459 in net_checksum_calculate /home/pm215/qemu/build/clang/../../net/checksum.c:78:47
#1 0x561f20117bfa in gem_transmit /home/pm215/qemu/build/clang/../../hw/net/cadence_gem.c:1386:21
#2 0x561f20115c61 in gem_write /home/pm215/qemu/build/clang/../../hw/net/cadence_gem.c:1650:13
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../net/checksum.c:78:47
Fix this by marking the eth_header struct as QEMU_PACKED, so that the
compiler knows it might be unaligned and will generate the right code
for accessing fields.
This is similar to commit f8b94b4 ("net: mark struct ip_header as
QEMU_PACKED") where we fixed this for a different struct defined in
this file.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Message-ID: <20260212140917.1443253-4-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The eth_header is not actually guaranteed to be aligned. We attempt
to deal with this in some places such as net_checksum_calculate() by
using lduw_be_p() and so on to access the fields, but this is not
sufficient to be correct, because even accessing a byte member within
a misaligned struct is undefined behaviour. The clang sanitizer will
emit an error like this if you run the sifive_u_mmc functional test
with sanitizers enabled:
../../net/checksum.c:168:24: runtime error: member access within misaligned address 0x5b7a7f829033 for type 'udp_header' (aka 'struct udp_header'), which requires 2 byte alignment
0x5b7a7f829033: note: pointer points here
ff ff ff ff 00 44 00 43 01 34 58 54 01 01 06 00 85 95 80 60 00 00 00 00 00 00 00 00 00 00 00 00
^
#0 0x5b7a71a5887e in net_checksum_calculate /home/pm215/qemu/build/clang/../../net/checksum.c:168:24
#1 0x5b7a7156819a in gem_transmit /home/pm215/qemu/build/clang/../../hw/net/cadence_gem.c:1386:21
#2 0x5b7a71566201 in gem_write /home/pm215/qemu/build/clang/../../hw/net/cadence_gem.c:1650:13
Fix this by marking the udp_header struct as QEMU_PACKED,
so that the compiler knows it might be unaligned and will
generate the right code for accessing fields.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Message-ID: <20260212140917.1443253-5-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The eth_header is not actually guaranteed to be aligned. We attempt
to deal with this in some places such as net_checksum_calculate() by
using lduw_be_p() and so on to access the fields, but this is not
sufficient to be correct, because even accessing a byte member within
a misaligned struct is undefined behaviour. The clang sanitizer will
emit an error like this if you run the sifive_u_mmc functional test
with sanitizers enabled:
../../net/checksum.c:144:24: runtime error: member access within misaligned address 0x619a74c32033 for type 'tcp_header' (aka 'struct tcp_header'), which requires 4 byte alignment
0x619a74c32033: note: pointer points here
0a 00 02 02 86 aa 00 16 52 c1 d3 70 00 00 00 00 a0 02 fa f0 00 00 00 00 02 04 05 b4 04 02 08 0a
^
#0 0x619a6ba84794 in net_checksum_calculate /home/pm215/qemu/build/clang/../../net/checksum.c:144:24
#1 0x619a6b5940da in gem_transmit /home/pm215/qemu/build/clang/../../hw/net/cadence_gem.c:1386:21
#2 0x619a6b592141 in gem_write /home/pm215/qemu/build/clang/../../hw/net/cadence_gem.c:1650:13
Fix this by marking the tcp_header struct as QEMU_PACKED.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Message-ID: <20260212140917.1443253-6-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
Fedora QA reported the following kernel panic: BUG: unable to handle page fault for address: 0000000040003e54 #PF: supervisor write access in kernel mode #PF: error_code(0x0002) - not-present page PGD 1082ec067 P4D 0 Oops: Oops: 0002 [#1] SMP NOPTI CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.19.0-0.rc4.260108gf0b9d8eb98df.34.fc43.x86_64 #1 PREEMPT(lazy) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS edk2-20251119-3.fc43 11/19/2025 RIP: 0010:vmware_hypercall4.constprop.0+0x52/0x90 Code: 48 83 c4 20 5b e9 69 f0 fc fe 8b 05 a0 c1 b2 01 85 c0 74 23 b8 68 58 4d 56 b9 27 00 00 00 31 d2 bb 04 00 00 00 66 ba 58 56 ed <89> 1f 89 0e 41 89 10 5b e9 3c f0 fc fe 6a 00 49 89 f9 45 31 c0 31 RSP: 0018:ff5eeb3240003e40 EFLAGS: 00010046 RAX: 0000000000000000 RBX: 000000000000ffca RCX: 000000000000ffac RDX: 0000000000000000 RSI: 0000000040003e58 RDI: 0000000040003e54 RBP: ff1e05f3c1204800 R08: ff5eeb3240003e5c R09: 000000009d899c41 R10: 000000000000003d R11: ff5eeb3240003ff8 R12: 0000000000000000 R13: 00000000000000ff R14: ff1e05f3c02f9e00 R15: 000000000000000c FS: 0000000000000000(0000) GS:ff1e05f489e40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000040003e54 CR3: 000000010841d002 CR4: 0000000000771ef0 PKRU: 55555554 Call Trace: <IRQ> vmmouse_report_events+0x13e/0x1b0 psmouse_handle_byte+0x15/0x60 ps2_interrupt+0x8a/0xd0 ... It was triggered by dereferencing a bad pointer (RDI) immediately after a VMware hypercall for VMWARE_CMD_ABSPOINTER_DATA in the vmmouse driver: ffffffff82135070 <vmware_hypercall4.constprop.0>: ... ffffffff821350ac: b8 68 58 4d 56 mov $0x564d5868,%eax ffffffff821350b1: b9 27 00 00 00 mov $0x27,%ecx ffffffff821350b6: 31 d2 xor %edx,%edx ffffffff821350b8: bb 04 00 00 00 mov $0x4,%ebx ffffffff821350bd: 66 ba 58 56 mov $0x5658,%dx ffffffff821350c1: ed in (%dx),%eax <-- hypercall ffffffff821350c2: 89 1f mov %ebx,(%rdi) <-- crash Reading the kernel disassembly shows that RDI should contain the value of a valid kernel stack address here (0xff5eeb3240003e54). Instead it contains 0x40003e54, suggesting the hypervisor cleared the upper 32 bits. And indeed, Alexey discovered that QEMU's vmmouse_get_data() and vmmouse_set_data() are only saving/restoring the lower 32 bits, while clearing the upper 32. Fix that by changing the type of the saved data array from uint32_t to uint64_t. Fixes: 548df2a ("VMMouse Emulation, by Anthony Liguori.") Reported-by: Justin Forbes <jforbes@fedoraproject.org> Debugged-by: Alexey Makhalov <alexey.makhalov@broadcom.com> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> Link: https://lore.kernel.org/r/c508fc1d4a4ccd8c9fb1e51b71df089e31115a53.1770309998.git.jpoimboe@kernel.org Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3293 Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
In plugin_exit() we call g_hash_table_get_values() to get a GList
which we look at to print some information. This code has
multiple issues:
* it names the local variable for the GList "count", which
shadows the "qemu_plugin_scoreboard *count". This isn't
incorrect, but it is unnecessarily confusing
* it doesn't free the list, and the leak sanitizer complains:
Indirect leak of 2328 byte(s) in 97 object(s) allocated from:
#0 0x5589b0b72293 in malloc (/home/pm215/qemu/build/x86-tgt-san/qemu-system-i386+0x1a2f293) (BuildId: 26964cad9e3f81d35fc144d7cc88b53adf6f60c7)
#1 0x78fd8cfa1ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x78fd8cf96e4a in g_list_prepend (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e4a) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
lowRISC#3 0x78fd8cf8b318 in g_hash_table_get_values (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c318) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
lowRISC#4 0x78fd84d1a90c in plugin_exit /home/pm215/qemu/build/x86-tgt-san/../../tests/tcg/plugins/mem.c:87:25
* in iterating through the list it updates "count", so by the
time we get to the end of the loop we no longer have a pointer
to the head of the list that we could use to free it
* it checks for the list being NULL twice (once in an if()
and once in the for() loop's "while" condition), which is
redundant
* it skips the loop if g_list_next(counts) is NULL, which means
it will wrongly skip the loop if the list has only one entry
Rewrite the iteration code to fix these problems.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20260305161531.1774895-3-peter.maydell@linaro.org
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
In patch_hwaddr() we allocate a GByteArray for the data we read back
from the guest; however we forget to free it, and the leak sanitizer
complains:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x56c00ad48293 in malloc (/home/pm215/qemu/build/x86-tgt-san/qemu-system-x86_64+0x1a9f293) (BuildId: 62e2a7dbe5ff146b2fa14d26e24e443f1967edd9)
#1 0x7b3e4cc91ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x7b3e4cc54c12 in g_array_sized_new (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x25c12) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
lowRISC#3 0x7b3e44b06b49 in patch_hwaddr /home/pm215/qemu/build/x86-tgt-san/../../tests/tcg/plugins/patch.c:68:29
Indirect leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x56c00ad486b0 in realloc (/home/pm215/qemu/build/x86-tgt-san/qemu-system-x86_64+0x1a9f6b0) (BuildId: 62e2a7dbe5ff146b2fa14d26e24e443f1967edd9)
#1 0x7b3e4cc92819 in g_realloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63819) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x7b3e4cc54b36 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x25b36) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
lowRISC#3 0x7b3e4cc55276 in g_array_set_size (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x26276) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
lowRISC#4 0x7b3e4cc55574 in g_byte_array_set_size (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x26574) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
lowRISC#5 0x56c00be2ccc1 in qemu_plugin_read_memory_hwaddr /home/pm215/qemu/build/x86-tgt-san/../../plugins/api.c:524:5
Mark the variable as g_autoptr(), as we already do in the equivalent
code in patch_vaddr().
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20260305161531.1774895-4-peter.maydell@linaro.org
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The QPCIDevice we get via qpci_device_foreach() is allocated
memory, and we need to g_free() it after use.
This fixes asan leaks like this:
Direct leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x622a5f16913d in calloc (/home/pm215/qemu/build/arm-clang/tests/qtest/iommu-smmuv3-test+0x1d413d) (BuildId: bc598be1f4ad6d1a9a600c55aeef36108bdb6a04)
#1 0x73ee41c0f771 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63771) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x622a5f1d4cec in qpci_device_find /home/pm215/qemu/build/arm-clang/../../tests/qtest/libqos/pci.c:82:11
lowRISC#3 0x622a5f1d4cec in qpci_device_foreach /home/pm215/qemu/build/arm-clang/../../tests/qtest/libqos/pci.c:34:19
lowRISC#4 0x622a5f23cc73 in setup_qtest_pci_device /home/pm215/qemu/build/arm-clang/../../tests/qtest/iommu-smmuv3-test.c:45:5
lowRISC#5 0x622a5f23cc73 in run_smmuv3_translation /home/pm215/qemu/build/arm-clang/../../tests/qtest/iommu-smmuv3-test.c:74:11
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20260302092225.4088227-5-peter.maydell@linaro.org
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The IOWatchPoll holds a reference to the iochannel while the "child"
source (iwp->src) is removed from the context and freed. Freeing the
source leads to the iochannel being also freed at
qio_channel_fd_source_finalize().
Later, io_watch_poll_prepare() tries to create another source with the
same iochannel and hits an use after free:
==8241==ERROR: AddressSanitizer: heap-use-after-free on address 0x514000000040
READ of size 8 at 0x514000000040 thread T2
#0 0x561c2d272fcd in object_get_class ../qom/object.c:1043:17
#1 0x561c2d338f84 in QIO_CHANNEL_GET_CLASS include/io/channel.h:29:1
#2 0x561c2d33b26f in qio_channel_create_watch ../io/channel.c:388:30
lowRISC#3 0x561c2d2f0993 in io_watch_poll_prepare ../chardev/char-io.c:65:20
...
0x514000000040 is located 0 bytes inside of 392-byte region [0x514000000040,0x5140000001c8)
freed by thread T2 here:
#0 0x561c2d2319a5 in free
#1 0x7fb2c0926638 in g_free
#2 0x561c2d276507 in object_finalize ../qom/object.c:734:9
lowRISC#3 0x561c2d271d0d in object_unref ../qom/object.c:1231:9
lowRISC#4 0x561c2d32ef1d in qio_channel_fd_source_finalize ../io/channel-watch.c:95:5
lowRISC#5 0x7fb2c091d124 in g_source_unref_internal ../glib/gmain.c:2298
lowRISC#6 0x561c2d2f0b6c in io_watch_poll_prepare ../chardev/char-io.c:71:9
...
previously allocated by thread T3 (connect) here:
#0 0x561c2d231c69 in malloc
#1 0x7fb2c0926518 in g_malloc
#2 0x561c2d27246e in object_new_with_type ../qom/object.c:767:15
lowRISC#3 0x561c2d272530 in object_new ../qom/object.c:789:12
lowRISC#4 0x561c2d320193 in qio_channel_socket_new ../io/channel-socket.c:64:31
lowRISC#5 0x561c2d308013 in tcp_chr_connect_client_async ../chardev/char-socket.c:1181:12
lowRISC#6 0x561c2d3002e7 in qmp_chardev_open_socket_client ../chardev/char-socket.c:1281:9
...
Fix the issue by incrementing the iochannel reference count when the
IOWatchPoll takes a reference and decrementing when it is finalized.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20260302092225.4088227-6-peter.maydell@linaro.org
[PMM: rebased]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
tcp_chr_free_connection() can be called multiple times in succession, in which case the yank function will get as argument a NULL s->sioc that has been cleared by the previous tcp_chr_free_connection() call. This leads to an abort() at yank_unregister_function(). #0 __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 __GI_abort () at abort.c:79 #2 qtest_check_status (s=0x513000005600) at ../tests/qtest/libqtest.c:209 lowRISC#3 qtest_wait_qemu (s=0x513000005600) at ../tests/qtest/libqtest.c:273 lowRISC#4 qtest_kill_qemu (s=0x513000005600) at ../tests/qtest/libqtest.c:285 lowRISC#5 kill_qemu_hook_func (s=0x513000005600) at ../tests/qtest/libqtest.c:294 lowRISC#6 g_hook_list_invoke (hook_list=0x55ea9cc750c0 <abrt_hooks>, may_recurse=0) at ../glib/ghook.c:534 lowRISC#7 sigabrt_handler (signo=6) at ../tests/qtest/libqtest.c:299 lowRISC#8 <signal handler called> lowRISC#9 __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:51 lowRISC#10 __GI_abort () at abort.c:79 lowRISC#11 yank_unregister_function (instance=0x7fb26f2ea9a0, func=0x55ea9bcc0a10 <char_socket_yank_iochannel>, opaque=0x0) at ../util/yank.c:151 lowRISC#12 tcp_chr_free_connection (chr=0x51300000ffc0) at ../chardev/char-socket.c:385 lowRISC#13 tcp_chr_disconnect_locked (chr=0x51300000ffc0) at ../chardev/char-socket.c:477 lowRISC#14 tcp_chr_disconnect (chr=0x51300000ffc0) at ../chardev/char-socket.c:495 lowRISC#15 tcp_chr_hup (channel=0x514000000040, cond=G_IO_HUP, opaque=0x51300000ffc0) at ../chardev/char-socket.c:536 lowRISC#16 qio_channel_fd_source_dispatch (source=0x50c0000b5fc0, callback=0x55ea9bcd6770 <tcp_chr_hup>, user_data=0x51300000ffc0) at ../io/channel-watch.c:84 lowRISC#17 g_main_dispatch (context=0x50f000000040) at ../glib/gmain.c:3381 lowRISC#18 g_main_context_dispatch (context=context@entry=0x50f000000040) at ../glib/gmain.c:4099 lowRISC#19 g_main_context_iterate (context=0x50f000000040, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4175 lowRISC#20 g_main_loop_run (loop=0x502000055690) at ../glib/gmain.c:4373 Commit ebae647 ("chardev: check if the chardev is registered for yanking") seems to have encountered a similar issue, but checking s->registered_yank is not a complete solution because that flag pertains to the yank instance, not to each individual function. Skip the yank_unregister_function() in case s->sioc is already NULL, which indicates the last yank function was already removed. Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20260302092225.4088227-7-peter.maydell@linaro.org [PMM: rebased] Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
Details: https://gitlab.com/qemu-project/qemu/-/issues/3144 The function schedule_next_request is called with tg->lock held and it may call throttle_group_co_restart_queue, which takes tgm->throttled_reqs_lock, qemu_co_mutex_lock may leave current coroutine if other iothread has taken the lock. If the next coroutine will call throttle_group_co_io_limits_intercept - it will try to take the mutex tg->lock which will never be released. Here is the backtrace of the iothread: Thread 30 (Thread 0x7f8aad1fd6c0 (LWP 24240) "IO iothread2"): #0 futex_wait (futex_word=0x5611adb7d828, expected=2, private=0) at ../sysdeps/nptl/futex-internal.h:146 #1 __GI___lll_lock_wait (futex=futex@entry=0x5611adb7d828, private=0) at lowlevellock.c:49 #2 0x00007f8ab5a97501 in lll_mutex_lock_optimized (mutex=0x5611adb7d828) at pthread_mutex_lock.c:48 lowRISC#3 ___pthread_mutex_lock (mutex=0x5611adb7d828) at pthread_mutex_lock.c:93 lowRISC#4 0x00005611823f5482 in qemu_mutex_lock_impl (mutex=0x5611adb7d828, file=0x56118289daca "../block/throttle-groups.c", line=372) at ../util/qemu-thread-posix.c:94 lowRISC#5 0x00005611822b0b39 in throttle_group_co_io_limits_intercept (tgm=0x5611af1bb4d8, bytes=4096, direction=THROTTLE_READ) at ../block/throttle-groups.c:372 lowRISC#6 0x00005611822473b1 in blk_co_do_preadv_part (blk=0x5611af1bb490, offset=15972311040, bytes=4096, qiov=0x7f8aa4000f98, qiov_offset=0, flags=BDRV_REQ_REGISTERED_BUF) at ../block/block-backend.c:1354 lowRISC#7 0x0000561182247fa0 in blk_aio_read_entry (opaque=0x7f8aa4005910) at ../block/block-backend.c:1619 lowRISC#8 0x000056118241952e in coroutine_trampoline (i0=-1543497424, i1=32650) at ../util/coroutine-ucontext.c:175 lowRISC#9 0x00007f8ab5a56f70 in ?? () at ../sysdeps/unix/sysv/linux/x86_64/__start_context.S:66 from target:/lib64/libc.so.6 lowRISC#10 0x00007f8aad1ef190 in ?? () lowRISC#11 0x0000000000000000 in ?? () The lock is taken in line 386: (gdb) p tg.lock $1 = {lock = {__data = {__lock = 2, __count = 0, __owner = 24240, __nusers = 1, __kind = 0, __spins = 0, __elision = 0, __list = {__prev = 0x0, __next = 0x0}}, __size = "\002\000\000\000\000\000\000\000\260^\000\000\001", '\000' <repeats 26 times>, __align = 2}, file = 0x56118289daca "../block/throttle-groups.c", line = 386, initialized = true} The solution is to use tg->lock to protect both ThreadGroup fields and ThrottleGroupMember.throttled_reqs. It doesn't seem to be possible to use separate locks because we need to first manipulate ThrottleGroup fields, then schedule next coroutine using throttled_reqs and after than update token field from ThrottleGroup depending on the throttled_reqs state. Signed-off-by: Dmitry Guryanov <dmitry.guryanov@gmail.com> Message-ID: <20251208085528.890098-1-dmitry.guryanov@gmail.com> Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The m68k mcf_intc interrupt controller currently implements its
inbound IRQ lines by calling qemu_allocate_irqs() in mcf_intc_init().
This results in leaks like this:
Direct leak of 2944 byte(s) in 46 object(s) allocated from:
#0 0x5cf95ec15323 in malloc (/home/pm215/qemu/build/san/qemu-system-m68k+0xf9e323) (BuildId: 18d55ef8ea9856e68ee30802078af5050b8b06c5)
#1 0x7637c65c5ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x5cf95f6b2f27 in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:767:15
lowRISC#3 0x5cf95f6aa62e in qemu_allocate_irq /home/pm215/qemu/build/san/../../hw/core/irq.c:91:25
lowRISC#4 0x5cf95f6aa62e in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:79:16
lowRISC#5 0x5cf95f5f6d99 in mcf5208evb_init /home/pm215/qemu/build/san/../../hw/m68k/mcf5208.c:310:11
This isn't an important leak, as it is memory we allocate once at
QEMU startup and that has to stay live for the lifetime of the
system. However it does point at a code improvement.
Modernise this to have the device itself create inbound GPIOs with
qdev_init_gpio_in() that the board can then refer to and wire up
individually.
As the device is used in only a single board, we can update device
and board in a single patch rather than having to try to figure out
some way to change the API more piecemeal.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Thomas Huth <th.huth+qemu@posteo.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20260306190425.3047580-1-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
In the sun4m machine init, we set up the cpu_irqs[] array
with the real inbound IRQs for each CPU, followed by some
dummy IRQs for the remaining slots from smp_cpus up to
MAX_CPUS. These dummy IRQs do nothing when set/cleared
because the dummy_cpu_set_irq() function does nothing.
Instead of creating these "do nothing" qemu_irqs, instead
pass the number of CPUs to slavio_intctl_init() so that it
can only wire up the interrupt controller's interrupts
for the CPUs that actually exist. Calling qemu_set_irq()
on an irq that isn't connected does nothing, so this is
a simpler way to achieve the same result.
This cleanup fixes an unimportant memory leak reported by
the address sanitizer that happens because we allocate these
dummy IRQs with qemu_allocate_irqs():
Direct leak of 1920 byte(s) in 15 object(s) allocated from:
#0 0x5cb7b120cf63 in malloc (/home/pm215/qemu/build/san/qemu-system-sparc+0xe0bf63) (BuildId: d27f9230a7cc82ebfaf0cf9e439dc215ddd7ac68)
#1 0x743cd6dc5ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x5cb7b1a42fb4 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
lowRISC#3 0x5cb7b19e7e72 in sun4m_hw_init /home/pm215/qemu/build/san/../../hw/sparc/sun4m.c:845:23
lowRISC#4 0x5cb7b141d3dd in machine_run_board_init /home/pm215/qemu/build/san/../../hw/core/machine.c:1709:5
lowRISC#5 0x5cb7b1542895 in qemu_init_board /home/pm215/qemu/build/san/../../system/vl.c:2717:5
lowRISC#6 0x5cb7b1542895 in qmp_x_exit_preconfig /home/pm215/qemu/build/san/../../system/vl.c:2811:5
lowRISC#7 0x5cb7b15493ac in qemu_init /home/pm215/qemu/build/san/../../system/vl.c:3849:9
lowRISC#8 0x5cb7b1f3f201 in main /home/pm215/qemu/build/san/../../system/main.c:71:5
lowRISC#9 0x743cd4a2a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
lowRISC#10 0x743cd4a2a28a in __libc_start_main csu/../csu/libc-start.c:360:3
lowRISC#11 0x5cb7b1172114 in _start (/home/pm215/qemu/build/san/qemu-system-sparc+0xd71114) (BuildId: d27f9230a7cc82ebfaf0cf9e439dc215ddd7ac68)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-ID: <20260307112931.3322532-2-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
In the sun4m machine creation code, we currently use qemu_allocate_irqs()
to set up the IRQ lines that act as the inbound IRQ lines to the CPUs.
This results in a memory leak:
Direct leak of 128 byte(s) in 1 object(s) allocated from:
#0 0x5a23c1281ec3 in malloc (/home/pm215/qemu/build/sparc-san/qemu-system-sparc+0xdf1ec3) (BuildId: e6aa10be01feb5524656dd083997bc82b85e3e93)
#1 0x79e8f78f0ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x5a23c1a94e54 in qemu_extend_irqs /home/pm215/qemu/build/sparc-san/../../hw/core/irq.c:77:51
lowRISC#3 0x5a23c1a39e03 in cpu_devinit /home/pm215/qemu/build/sparc-san/../../hw/sparc/sun4m.c:802:17
lowRISC#4 0x5a23c1a39e03 in sun4m_hw_init /home/pm215/qemu/build/sparc-san/../../hw/sparc/sun4m.c:838:9
The leak is unimportant as it is a "once at startup" leak, but
fixing it helps in getting a clean leak-sanitizer test run.
Switch the sun4m code to handle CPU interrupt lines in the same way
as the leon3 machine does: the machine init code uses
qdev_init_gpio_in to create GPIO lines on the CPU objects. This is a
little bit odd as ideally the CPU would do that itself, but for these
32-bit SPARC machines the machine and the CPU are very closely
coupled already (the functions handling the IRQ lines modify data
fields inside the CPU).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-ID: <20260307112931.3322532-3-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The slavio_timer device's instance_init function allocates memory for
TimerContext structs and a ptimer, but it never frees this memory, so
we will leak it if the QMP interface does introspection of this
device type, as reported by the clang address sanitizer:
Indirect leak of 4896 byte(s) in 17 object(s) allocated from:
#0 0x5f2948d9b14d in calloc (/home/pm215/qemu/build/san/qemu-system-sparc+0xe0c14d) (BuildId: 7210711bdf6f7fbd0b863bd2dfcc7c42c7175db1)
#1 0x758584b11771 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63771) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x5f2949097b8a in slavio_timer_init /home/pm215/qemu/build/san/../../hw/timer/slavio_timer.c:403:14
lowRISC#3 0x5f29495d790f in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
lowRISC#4 0x5f29495d96ef in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
lowRISC#5 0x5f2949a30a26 in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11
Indirect leak of 1632 byte(s) in 17 object(s) allocated from:
#0 0x5f2948d9b14d in calloc (/home/pm215/qemu/build/san/qemu-system-sparc+0xe0c14d) (BuildId: 7210711bdf6f7fbd0b863bd2dfcc7c42c7175db1)
#1 0x758584b11771 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63771) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x5f2948f7c65a in ptimer_init /home/pm215/qemu/build/san/../../hw/core/ptimer.c:464:9
lowRISC#3 0x5f2949097c1f in slavio_timer_init /home/pm215/qemu/build/san/../../hw/timer/slavio_timer.c:407:32
lowRISC#4 0x5f29495d790f in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
lowRISC#5 0x5f29495d96ef in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
lowRISC#6 0x5f2949a30a26 in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11
Avoid the TimerContext leaks by making them an array inside the
SLAVIO_TimerState struct instead of allocating a compile-time-fixed
number of them each individually with g_new0() and then throwing away
the pointer.
Avoid the ptimer() leak by calling ptimer_free in
instance_finalize().
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-ID: <20260307112931.3322532-4-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The current implementation of qdev_get_printable_name() sometimes
returns a string that must not be freed (vdev->id or the fixed
fallback string "<unknown device>" and sometimes returns a string
that must be freed (the return value of qdev_get_dev_path()). This
forces callers to leak the string in the "must be freed" case.
Make the function consistent that it always returns a string that
the caller must free, and make the three callsites free it.
This fixes leaks like this that show up when running "make check"
with the address sanitizer enabled:
Direct leak of 13 byte(s) in 1 object(s) allocated from:
#0 0x5561de21f293 in malloc (/home/pm215/qemu/build/san/qemu-system-i386+0x1a2d293) (BuildId: 6d6fad7130fd5c8dbbc03401df554f68b8034936)
#1 0x767ad7a82ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x5561deaf34f2 in pcibus_get_dev_path /home/pm215/qemu/build/san/../../hw/pci/pci.c:2792:12
lowRISC#3 0x5561df9d8830 in qdev_get_printable_name /home/pm215/qemu/build/san/../../hw/core/qdev.c:431:24
lowRISC#4 0x5561deebdca2 in virtio_init_region_cache /home/pm215/qemu/build/san/../../hw/virtio/virtio.c:298:17
lowRISC#5 0x5561df05f842 in memory_region_write_accessor /home/pm215/qemu/build/san/../../system/memory.c:491:5
lowRISC#6 0x5561df05ed1b in access_with_adjusted_size /home/pm215/qemu/build/san/../../system/memory.c:567:18
lowRISC#7 0x5561df05e3fa in memory_region_dispatch_write /home/pm215/qemu/build/san/../../system/memory.c
lowRISC#8 0x5561df0aa805 in address_space_stm_internal /home/pm215/qemu/build/san/../../system/memory_ldst.c.inc:85:13
lowRISC#9 0x5561df0bcad3 in qtest_process_command /home/pm215/qemu/build/san/../../system/qtest.c:480:13
Cc: qemu-stable@nongnu.org
Fixes: e209d4d ("virtio: improve virtqueue mapping error messages")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20260307155046.3940197-3-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The sifive_e_aon watchdog creates a timer with timer_new_ns() in its
instance_init method, but does not free it in instance_finalize.
This means that QMP introspection of the device leaks it:
Direct leak of 48 byte in 1 object allocated from:
#0 in calloc
#1 in g_malloc0
#2 in timer_new_full /home/pm215/qemu/include/qemu/timer.h:520:21
lowRISC#3 in timer_new /home/pm215/qemu/include/qemu/timer.h:543:12
lowRISC#4 in timer_new_ns /home/pm215/qemu/include/qemu/timer.h:563:12
lowRISC#5 in sifive_e_aon_init /home/pm215/qemu/build/san/../../hw/misc/sifive_e_aon.c:286:21
lowRISC#6 in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
lowRISC#7 in object_initialize /home/pm215/qemu/build/san/../../qom/object.c:578:5
lowRISC#8 in object_initialize_child_with_propsv /home/pm215/qemu/build/san/../../qom/object.c:608:5
lowRISC#9 in object_initialize_child_with_props /home/pm215/qemu/build/san/../../qom/object.c:591:10
lowRISC#10 in object_initialize_child_internal /home/pm215/qemu/build/san/../../qom/object.c:645:5
lowRISC#11 in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
lowRISC#12 in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
lowRISC#13 in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11
Allocating a separate QEMUTimer with timer_new() is not the preferred
interface (per the comments in include/qemu/timer.h); switch to an
inline struct initialized with timer_init(), which we can clean up
with timer_del() in finalize.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20260309095129.1406506-1-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Mar 17, 2026
The TYPE_RISCV_CPC device allocates an array in its instance_init,
but does not free this, leading to leaks like this from QOM/QMP
introspection:
Direct leak of 512 byte in 1 object allocated from:
#0 in calloc
#1 in g_malloc0
#2 in riscv_cpc_init /home/pm215/qemu/build/san/../../hw/misc/riscv_cpc.c:175:15
lowRISC#3 in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
lowRISC#4 in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
lowRISC#5 in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11
lowRISC#6 in qdev_device_help /home/pm215/qemu/build/san/../../system/qdev-monitor.c:313:17
lowRISC#7 in hmp_device_add /home/pm215/qemu/build/san/../../system/qdev-monitor.c:1005:9
Free the array in instance_finalize.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20260309095328.1406779-1-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Apr 24, 2026
When virtio_gpu_rutabaga_get_num_capsets() returns 0, virtio_init() isn't called and the device later crashes during realize. ==72545==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x556ad6f7ba9e bp 0x7ffe6958f810 sp 0x7ffe6958f7e0 T0) ==72545==The signal is caused by a READ memory access. ==72545==Hint: address points to the zero page. #0 0x556ad6f7ba9e in virtio_memory_listener_commit ../hw/virtio/virtio.c:4034 #1 0x556ad6a24c96 in listener_add_address_space ../system/memory.c:3128 #2 0x556ad6a25d15 in memory_listener_register ../system/memory.c:3216 lowRISC#3 0x556ad6f7bf11 in virtio_device_realize ../hw/virtio/virtio.c:4075 Rework error handling of the function to set Error appropriately. 0 capset may be ok now. Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Apr 24, 2026
Recent fixes to TLS tasks memory handling have left the TLS bye task
uncovered. Fix by freeing the task in the same way the handshake task
is freed.
Direct leak of 704 byte(s) in 4 object(s) allocated from:
#1 0x7f5909b1d6a0 in g_malloc0 ../glib/gmem.c:163
#2 0x557650496d61 in qio_task_new ../io/task.c:58:12
lowRISC#3 0x557650475d7f in qio_channel_tls_bye ../io/channel-tls.c:352:12
lowRISC#4 0x55764f7a1bb4 in migration_tls_channel_end ../migration/tls.c:159:5
lowRISC#5 0x55764f709750 in migration_ioc_shutdown_gracefully ../migration/multifd.c:462:9
lowRISC#6 0x55764f6fcf53 in multifd_send_terminate_threads ../migration/multifd.c:493:13
lowRISC#7 0x55764f6fcafb in multifd_send_shutdown ../migration/multifd.c:580:5
lowRISC#8 0x55764f6e1b14 in migration_cleanup ../migration/migration.c:1323:9
lowRISC#9 0x55764f6f5bac in migration_cleanup_bh ../migration/migration.c:1350:5
Fixes: d39d0f3 ("io: fix cleanup for TLS I/O source data on cancellation")
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Daniel P. Berrangé <berrange@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260311213418.16951-3-farosas@suse.de
Signed-off-by: Fabiano Rosas <farosas@suse.de>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Apr 24, 2026
Some tests can cause QEMU to exit(1) too early while the incoming
coroutine has not yielded for a first time yet. This trips ASAN
because resources related to dispatching the incoming process will
still be allocated in the io/channel.c layer without a
straight-forward way for the migration code to clean them up.
As an example of one such issue, the UUID validation happens early
enough that the temporary socket from qio_net_listener_channel_func()
still has an elevated refcount. If it fails, the listener dispatch
code never gets to free the resource:
Direct leak of 400 byte(s) in 1 object(s) allocated from:
#0 0x55e668890a07 in malloc asan_malloc_linux.cpp:68:3
#1 0x7f3c7e2b6648 in g_malloc ../glib/gmem.c:130
#2 0x55e66a8ef05f in object_new_with_type ../qom/object.c:767:15
lowRISC#3 0x55e66a8ef178 in object_new ../qom/object.c:789:12
lowRISC#4 0x55e66a93bcc6 in qio_channel_socket_new ../io/channel-socket.c:70:31
lowRISC#5 0x55e66a93f34f in qio_channel_socket_accept ../io/channel-socket.c:401:12
lowRISC#6 0x55e66a96752a in qio_net_listener_channel_func ../io/net-listener.c:64:12
lowRISC#7 0x55e66a94bdac in qio_channel_fd_source_dispatch ../io/channel-watch.c:84:12
lowRISC#8 0x7f3c7e2adf4b in g_main_dispatch ../glib/gmain.c:3476
lowRISC#9 0x7f3c7e2adf4b in g_main_context_dispatch_unlocked ../glib/gmain.c:4284
lowRISC#10 0x7f3c7e2b00c8 in g_main_context_dispatch ../glib/gmain.c:4272
The exit(1) also requires some tests to setup qtest to expect a return
code of 1 from the QEMU process. Although we can check migration
status changes to be fairly certain where the failure happened, there
is always the possibility of QEMU exiting for another reason and the
test passing. This happens frequently with sanitizers enabled, but
also risks masking issues in the regular build.
Stop allowing the incoming migration to exit and instead require the
tests to wait for the FAILED state and end QEMU gracefully with
qtest_quit.
In practice this means setting exit-on-error=false for every incoming
migration, changing MIG_TEST_FAIL_DEST_QUIT_ERR to MIG_TEST_FAIL and
waiting for a change of state where necessary.
With this, the MIG_TEST_FAIL_DEST_QUIT_ERR error result is now unused,
remove it.
The affected tests are:
validate_uuid_error
multifd_tcp_cancel
dirty_limit
precopy_unix_tls_x509_default_host
precopy_tcp_tls_no_hostname
tcp_tls_x509_mismatch_host
dbus_vmstate_missing_src
dbus_vmstate_missing_dst
Also add a comment to QEMU source explaining that the incoming
coroutine might block for a while until it yields as this is the
actual root cause of the issue.
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Link: https://lore.kernel.org/qemu-devel/20260311213418.16951-6-farosas@suse.de
[assert that key doesn't already exists]
Signed-off-by: Fabiano Rosas <farosas@suse.de>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Apr 24, 2026
…lize
The riscv-iommu device makes various allocations in its
instance_init method. These will leak when QMP inits an
object of this type to introspect it, as can be seen if you
run 'make check' with the address sanitizer enabled:
Direct leak of 4096 byte(s) in 1 object(s) allocated from:
#0 0x5d8415b6ed9d in calloc (/home/pm215/qemu/build/san/qemu-system-riscv32+0x1832d9d) (BuildId: fedcc313e48ba803d63837329c37fd609dd50849)
#1 0x75c0502f1771 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x63771) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x5d8416d09391 in riscv_iommu_instance_init /home/pm215/qemu/build/san/../../hw/riscv/riscv-iommu.c:2463:18
lowRISC#3 0x5d841710483f in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
lowRISC#4 0x5d8417104ee9 in object_initialize /home/pm215/qemu/build/san/../../qom/object.c:578:5
lowRISC#5 0x5d8417104ee9 in object_initialize_child_with_propsv /home/pm215/qemu/build/san/../../qom/object.c:608:5
lowRISC#6 0x5d8417104db1 in object_initialize_child_with_props /home/pm215/qemu/build/san/../../qom/object.c:591:10
lowRISC#7 0x5d8417106506 in object_initialize_child_internal /home/pm215/qemu/build/san/../../qom/object.c:645:5
lowRISC#8 0x5d8416d16a12 in riscv_iommu_sys_init /home/pm215/qemu/build/san/../../hw/riscv/riscv-iommu-sys.c:199:5
lowRISC#9 0x5d841710483f in object_initialize_with_type /home/pm215/qemu/build/san/../../qom/object.c:570:5
lowRISC#10 0x5d841710661f in object_new_with_type /home/pm215/qemu/build/san/../../qom/object.c:774:5
lowRISC#11 0x5d841755d956 in qmp_device_list_properties /home/pm215/qemu/build/san/../../qom/qom-qmp-cmds.c:206:11
(and other similar backtraces).
Fix these by freeing the resources we allocate in instance_init in
instance_finalize. In some cases we were freeing these in unrealize,
and in some cases not at all.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20260307125222.3656140-1-peter.maydell@linaro.org>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
lexbaileylowrisc
pushed a commit
that referenced
this pull request
Apr 24, 2026
The pci_piix_realize() function's use of qemu_allocate_irqs()
results in a memory leak:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x61045c7a1a43 in malloc (/home/pm215/qemu/build/san/qemu-system-mips+0x16f8a43) (BuildId: aa43d3865e0f1991b1fc04422b5570fe522b6fa7)
#1 0x724cc3095ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
#2 0x61045db72134 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
lowRISC#3 0x61045cd7bf49 in pci_piix_realize /home/pm215/qemu/build/san/../../hw/isa/piix.c:318:35
lowRISC#4 0x61045cf4533e in pci_qdev_realize /home/pm215/qemu/build/san/../../hw/pci/pci.c:2308:9
lowRISC#5 0x61045db6cbca in device_set_realized /home/pm215/qemu/build/san/../../hw/core/qdev.c:523:13
lowRISC#6 0x61045db86bd9 in property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:2376:5
lowRISC#7 0x61045db81c5e in object_property_set /home/pm215/qemu/build/san/../../qom/object.c:1450:5
lowRISC#8 0x61045db8e2fc in object_property_set_qobject /home/pm215/qemu/build/san/../../qom/qom-qobject.c:28:10
lowRISC#9 0x61045db8258f in object_property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:1520:15
lowRISC#10 0x61045db687aa in qdev_realize_and_unref /home/pm215/qemu/build/san/../../hw/core/qdev.c:283:11
lowRISC#11 0x61045d892e21 in mips_malta_init /home/pm215/qemu/build/san/../../hw/mips/malta.c:1239:5
(The i386 PC sets the has-pic property to 'false', so this only
affects the MIPS Malta board.)
Fix this by embedding the i8259 irq in the device state instead of
allocating it. This is a similar fix to the one we used for vt82c686
in commit 2225dc5, except that we use qemu_init_irq_child()
instead of qemu_init_irq(). The behaviour is identical except that
the _child() version avoids what would be a leak if we ever
unrealized the device.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
Message-id: 20260309171258.1905205-1-peter.maydell@linaro.org
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE
see the updated one here: lowRISC#323