util/interval-tree: Use qatomic_set_mb in rb_link_node
Ensure that the stores to rb_left and rb_right are complete before inserting the new node into the tree. Otherwise a concurrent reader could see garbage in the new leaf. Cc: qemu-stable@nongnu.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> (cherry picked from commit 4c8baa0) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> (Mjt: s/qatomic_set_mb/qatomic_mb_set/ for 8.0 - it was renamed later)
target/ppc: Disable goto_tb with architectural singlestep
The change to use translator_use_goto_tb went too far, as the CF_SINGLE_STEP flag managed by the translator only handles gdb single stepping and not the architectural single stepping modeled in DisasContext.singlestep_enabled. Fixes: 6e9cc37 ("target/ppc: Use translator_use_goto_tb") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1795 Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> (cherry picked from commit 2e718e6) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
linux-user/armeb: Fix __kernel_cmpxchg() for armeb
Commit 7f4f0d9 ("linux-user/arm: Implement __kernel_cmpxchg with host atomics") switched to use qatomic_cmpxchg() to swap a word with the memory content, but missed to endianess-swap the oldval and newval values when emulating an armeb CPU, which expects words to be stored in big endian in the guest memory. The bug can be verified with qemu >= v7.0 on any little-endian host, when starting the armeb binary of the upx program, which just hangs without this patch. Cc: qemu-stable@nongnu.org Signed-off-by: Helge Deller <deller@gmx.de> Reported-by: "Markus F.X.J. Oberhumer" <markus@oberhumer.com> Reported-by: John Reiser <jreiser@BitWagon.com> Closes: upx/upx#687 Message-Id: <ZMQVnqY+F+5sTNFd@p100> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> (cherry picked from commit 38dd78c) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
thread-pool: signal "request_cond" while locked
thread_pool_free() might have been called on the `pool`, which would
be a reason for worker_thread() to quit. In this case,
`pool->request_cond` is been destroyed.
If worker_thread() didn't managed to signal `request_cond` before it
been destroyed by thread_pool_free(), we got:
util/qemu-thread-posix.c:198: qemu_cond_signal: Assertion `cond->initialized' failed.
One backtrace:
__GI___assert_fail (assertion=0x55555614abcb "cond->initialized", file=0x55555614ab88 "util/qemu-thread-posix.c", line=198,
function=0x55555614ad80 <__PRETTY_FUNCTION__.17104> "qemu_cond_signal") at assert.c:101
qemu_cond_signal (cond=0x7fffb800db30) at util/qemu-thread-posix.c:198
worker_thread (opaque=0x7fffb800dab0) at util/thread-pool.c:129
qemu_thread_start (args=0x7fffb8000b20) at util/qemu-thread-posix.c:505
start_thread (arg=<optimized out>) at pthread_create.c:486
Reported here:
https://lore.kernel.org/all/ZJwoK50FcnTSfFZ8@MacBook-Air-de-Roger.local/T/#u
To avoid issue, keep lock while sending a signal to `request_cond`.
Fixes: 900fa20 ("thread-pool: replace semaphore with condition variable")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20230714152720.5077-1-anthony.perard@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
(cherry picked from commit f4f7136)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>xen-block: Avoid leaks on new error path
Commit 1898293 ("xen-block: Use specific blockdev driver") introduced a new error path, without taking care of allocated resources. So only allocate the qdicts after the error check, and free both `filename` and `driver` when we are about to return and thus taking care of both success and error path. Coverity only spotted the leak of qdicts (*_layer variables). Reported-by: Peter Maydell <peter.maydell@linaro.org> Fixes: Coverity CID 1508722, 1398649 Fixes: 1898293 ("xen-block: Use specific blockdev driver") Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20230704171819.42564-1-anthony.perard@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> (cherry picked from commit aa36243) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
io: remove io watch if TLS channel is closed during handshake
The TLS handshake make take some time to complete, during which time an I/O watch might be registered with the main loop. If the owner of the I/O channel invokes qio_channel_close() while the handshake is waiting to continue the I/O watch must be removed. Failing to remove it will later trigger the completion callback which the owner is not expecting to receive. In the case of the VNC server, this results in a SEGV as vnc_disconnect_start() tries to shutdown a client connection that is already gone / NULL. CVE-2023-3354 Reported-by: jiangyegen <jiangyegen@huawei.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> (cherry picked from commit 10be627) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
hw/xen: fix off-by-one in xen_evtchn_set_gsi()
Coverity points out (CID 1508128) a bounds checking error. We need to check for gsi >= IOAPIC_NUM_PINS, not just greater-than. Also fix up an assert() that has the same problem, that Coverity didn't see. Fixes: 4f81baa ("hw/xen: Support GSI mapping to PIRQ") Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-Id: <20230801175747.145906-2-dwmw2@infradead.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> (cherry picked from commit cf885b1) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
target/nios2: Pass semihosting arg to exit
Instead of using R_ARG0 (the semihost function number), use R_ARG1 (the provided exit status). Signed-off-by: Keith Packard <keithp@keithp.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20230801152245.332749-1-keithp@keithp.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> (cherry picked from commit c11d5bd) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
target/nios2: Fix semihost lseek offset computation
The arguments for deposit64 are (value, start, length, fieldval); this appears to have thought they were (value, fieldval, start, length). Reorder the parameters to match the actual function. Signed-off-by: Keith Packard <keithp@keithp.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Fixes: d1e23cb ("target/nios2: Use semihosting/syscalls.h") Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20230731235245.295513-1-keithp@keithp.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> (cherry picked from commit 71e2dd6) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
target/m68k: Fix semihost lseek offset computation
The arguments for deposit64 are (value, start, length, fieldval); this appears to have thought they were (value, fieldval, start, length). Reorder the parameters to match the actual function. Cc: qemu-stable@nongnu.org Fixes: 9502725 ("target/m68k: Use semihosting/syscalls.h") Reported-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-Id: <20230801154519.3505531-1-peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> (cherry picked from commit 8caaae7) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
hw/virtio-iommu: Fix potential OOB access in virtio_iommu_handle_comm…
…and() In the virtio_iommu_handle_command() when a PROBE request is handled, output_size takes a value greater than the tail size and on a subsequent iteration we can get a stack out-of-band access. Initialize the output_size on each iteration. The issue was found with ASAN. Credits to: Yiming Tao(Zhejiang University) Gaoning Pan(Zhejiang University) Fixes: 1733eeb ("virtio-iommu: Implement RESV_MEM probe request") Signed-off-by: Eric Auger <eric.auger@redhat.com> Reported-by: Mauro Matteo Cascella <mcascell@redhat.com> Cc: qemu-stable@nongnu.org Message-Id: <20230717162126.11693-1-eric.auger@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> (cherry picked from commit cf2f89e) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
When the vhost-user reconnect to the backend, the notifer should be cleanup. Otherwise, the fd resource will be exhausted. Fixes: f9a09ca ("vhost: add support for configure interrupt") Signed-off-by: Li Feng <fengli@smartx.com> Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com> Message-Id: <20230731121018.2856310-2-fengli@smartx.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Tested-by: Fiona Ebner <f.ebner@proxmox.com> (cherry picked from commit 18f2971) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
virtio-crypto: verify src&dst buffer length for sym request
For symmetric algorithms, the length of ciphertext must be as same as the plaintext. The missing verification of the src_len and the dst_len in virtio_crypto_sym_op_helper() may lead buffer overflow/divulged. This patch is originally written by Yiming Tao for QEMU-SECURITY, resend it(a few changes of error message) in qemu-devel. Fixes: CVE-2023-3180 Fixes: 04b9b37("virtio-crypto: add data queue processing handler") Cc: Gonglei <arei.gonglei@huawei.com> Cc: Mauro Matteo Cascella <mcascell@redhat.com> Cc: Yiming Tao <taoym@zju.edu.cn> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Message-Id: <20230803024314.29962-2-pizhenwei@bytedance.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> (cherry picked from commit 9d38a84) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
cryptodev: Handle unexpected request to avoid crash
Generally guest side should discover which services the device is able to offer, then do requests on device. However it's also possible to break this rule in a guest. Handle unexpected request here to avoid NULL pointer dereference. Fixes: e7a775f ('cryptodev: Account statistics') Cc: Gonglei <arei.gonglei@huawei.com> Cc: Mauro Matteo Cascella <mcascell@redhat.com> Cc: Xiao Lei <nop.leixiao@gmail.com> Cc: Yongkang Jia <kangel@zju.edu.cn> Reported-by: Yiming Tao <taoym@zju.edu.cn> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Message-Id: <20230803024314.29962-3-pizhenwei@bytedance.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> (cherry picked from commit 15b11a1) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
target/hppa: Move iaoq registers and thus reduce generated code size
On hppa the Instruction Address Offset Queue (IAOQ) registers specifies the next to-be-executed instructions addresses. Each generated TB writes those registers at least once, so those registers are used heavily in generated code. Looking at the generated assembly, for a x86-64 host this code to write the address $0x7ffe826f into iaoq_f is generated: 0x7f73e8000184: c7 85 d4 01 00 00 6f 82 movl $0x7ffe826f, 0x1d4(%rbp) 0x7f73e800018c: fe 7f 0x7f73e800018e: c7 85 d8 01 00 00 73 82 movl $0x7ffe8273, 0x1d8(%rbp) 0x7f73e8000196: fe 7f With the trivial change, by moving the variables iaoq_f and iaoq_b to the top of struct CPUArchState, the offset to %rbp is reduced (from 0x1d4 to 0), which allows the x86-64 tcg to generate 3 bytes less of generated code per move instruction: 0x7fc1e800018c: c7 45 00 6f 82 fe 7f movl $0x7ffe826f, (%rbp) 0x7fc1e8000193: c7 45 04 73 82 fe 7f movl $0x7ffe8273, 4(%rbp) Overall this is a reduction of generated code (not a reduction of number of instructions). A test run with checks the generated code size by running "/bin/ls" with qemu-user shows that the code size shrinks from 1616767 to 1569273 bytes, which is ~97% of the former size. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Helge Deller <deller@gmx.de> Cc: qemu-stable@nongnu.org (cherry picked from commit f8c0fd9) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
pci: do not respond config requests after PCI device eject
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2224964 In migration with VF failover, Windows guest and ACPI hot unplug we do not need to satisfy config requests, otherwise the guest immediately detects the device and brings up its driver. Many network VF's are stuck on the guest PCI bus after the migration. Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com> Message-Id: <20230728084049.191454-1-yuri.benditovich@daynix.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> (cherry picked from commit 348e354) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
hw/i386/intel_iommu: Fix trivial endianness problems
After reading the guest memory with dma_memory_read(), we have to make sure that we byteswap the little endian data to the host's byte order. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20230802135723.178083-2-thuth@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Peter Xu <peterx@redhat.com> (cherry picked from commit cc2a084) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
hw/i386/intel_iommu: Fix endianness problems related to VTD_IR_TableE…
…ntry The code already tries to do some endianness handling here, but currently fails badly: - While it already swaps the data when logging errors / tracing, it fails to byteswap the value before e.g. accessing entry->irte.present - entry->irte.source_id is swapped with le32_to_cpu(), though this is a 16-bit value - The whole union is apparently supposed to be swapped via the 64-bit data[2] array, but the struct is a mixture between 32 bit values (the first 8 bytes) and 64 bit values (the second 8 bytes), so this cannot work as expected. Fix it by converting the struct to two proper 64-bit bitfields, and by swapping the values only once for everybody right after reading the data from memory. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20230802135723.178083-3-thuth@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> (cherry picked from commit 642ba89) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
hw/i386/intel_iommu: Fix struct VTDInvDescIEC on big endian hosts
On big endian hosts, we need to reverse the bitfield order in the struct VTDInvDescIEC, just like it is already done for the other bitfields in the various structs of the intel-iommu device. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20230802135723.178083-4-thuth@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> (cherry picked from commit 4572b22) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
hw/i386/intel_iommu: Fix index calculation in vtd_interrupt_remap_msi()
The values in "addr" are populated locally in this function in host endian byte order, so we must not swap the index_l field here. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20230802135723.178083-5-thuth@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Peter Xu <peterx@redhat.com> (cherry picked from commit fcd8027) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
hw/i386/x86-iommu: Fix endianness issue in x86_iommu_irq_to_msi_messa…
…ge() The values in "msg" are assembled in host endian byte order (the other field are also not swapped), so we must not swap the __addr_head here. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20230802135723.178083-6-thuth@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Peter Xu <peterx@redhat.com> (cherry picked from commit 37cf5ce) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
include/hw/i386/x86-iommu: Fix struct X86IOMMU_MSIMessage for big end…
…ian hosts The first bitfield here is supposed to be used as a 64-bit equivalent to the "uint64_t msi_addr" in the union. To make this work correctly on big endian hosts, too, the __addr_hi field has to be part of the bitfield, and the the bitfield members must be declared with "uint64_t" instead of "uint32_t" - otherwise the values are placed in the wrong bytes on big endian hosts. Same applies to the 32-bit "msi_data" field: __resved1 must be part of the bitfield, and the members must be declared with "uint32_t" instead of "uint16_t". Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20230802135723.178083-7-thuth@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> (cherry picked from commit e1e56c0) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>