Skip to content
Snippets Groups Projects
  1. Apr 08, 2022
    • Wentao Liang's avatar
      virtio-iommu: use-after-free fix · 4bf58c72
      Wentao Liang authored
      
      A potential Use-after-free was reported in virtio_iommu_handle_command
      when using virtio-iommu:
      
      > I find a potential Use-after-free in QEMU 6.2.0, which is in
      > virtio_iommu_handle_command() (./hw/virtio/virtio-iommu.c).
      >
      >
      > Specifically, in the loop body, the variable 'buf' allocated at line 639 can be
      > freed by g_free() at line 659. However, if the execution path enters the loop
      > body again and the if branch takes true at line 616, the control will directly
      > jump to 'out' at line 651. At this time, 'buf' is a freed pointer, which is not
      > assigned with an allocated memory but used at line 653. As a result, a UAF bug
      > is triggered.
      >
      >
      >
      > 599     for (;;) {
      > ...
      > 615         sz = iov_to_buf(iov, iov_cnt, 0, &head, sizeof(head));
      > 616         if (unlikely(sz != sizeof(head))) {
      > 617             tail.status = VIRTIO_IOMMU_S_DEVERR;
      > 618             goto out;
      > 619         }
      > ...
      > 639             buf = g_malloc0(output_size);
      > ...
      > 651 out:
      > 652         sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
      > 653                           buf ? buf : &tail, output_size);
      > ...
      > 659         g_free(buf);
      >
      > We can fix it by set ‘buf‘ to NULL after freeing it:
      >
      >
      > 651 out:
      > 652         sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
      > 653                           buf ? buf : &tail, output_size);
      > ...
      > 659         g_free(buf);
      > +++ buf = NULL;
      > 660     }
      
      Fix as suggested by the reporter.
      
      Signed-off-by: default avatarWentao Liang <Wentao_Liang_g@163.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Message-id: 20220407095047.50371-1-mst@redhat.com
      Message-ID: <20220406040445-mutt-send-email-mst@kernel.org>
      Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
      4bf58c72
  2. Apr 07, 2022
  3. Apr 06, 2022
  4. Apr 05, 2022
  5. Apr 04, 2022
    • Daniel Henrique Barboza's avatar
      hw/ppc: free env->tb_env in spapr_unrealize_vcpu() · ef95a244
      Daniel Henrique Barboza authored
      
      The timebase is allocated during spapr_realize_vcpu() and it's not
      freed. This results in memory leaks when doing vcpu unplugs:
      
      ==636935==
      ==636935== 144 (96 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 6
      ,461 of 8,135
      ==636935==    at 0x4897468: calloc (vg_replace_malloc.c:760)
      ==636935==    by 0x5077213: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6400.4)
      ==636935==    by 0x507757F: g_malloc0_n (in /usr/lib64/libglib-2.0.so.0.6400.4)
      ==636935==    by 0x93C3FB: cpu_ppc_tb_init (ppc.c:1066)
      ==636935==    by 0x97BC2B: spapr_realize_vcpu (spapr_cpu_core.c:268)
      ==636935==    by 0x97C01F: spapr_cpu_core_realize (spapr_cpu_core.c:337)
      ==636935==    by 0xD4626F: device_set_realized (qdev.c:531)
      ==636935==    by 0xD55273: property_set_bool (object.c:2273)
      ==636935==    by 0xD523DF: object_property_set (object.c:1408)
      ==636935==    by 0xD588B7: object_property_set_qobject (qom-qobject.c:28)
      ==636935==    by 0xD52897: object_property_set_bool (object.c:1477)
      ==636935==    by 0xD4579B: qdev_realize (qdev.c:333)
      ==636935==
      
      This patch adds a cpu_ppc_tb_free() helper in hw/ppc/ppc.c to allow us
      to free the timebase. This leak is then solved by calling
      cpu_ppc_tb_free() in spapr_unrealize_vcpu().
      
      Fixes: 6f4b5c3e ("spapr: CPU hot unplug support")
      Signed-off-by: default avatarDaniel Henrique Barboza <danielhb413@gmail.com>
      Reviewed-by: default avatarCédric Le Goater <clg@kaod.org>
      Reviewed-by: default avatarDavid Gibson <david@gibson.dropbear.id.au>
      Message-Id: <20220329124545.529145-2-danielhb413@gmail.com>
      Signed-off-by: default avatarCédric Le Goater <clg@kaod.org>
      ef95a244
  6. Apr 01, 2022
    • Will Cohen's avatar
      9p: move P9_XATTR_SIZE_MAX from 9p.h to 9p.c · a136d175
      Will Cohen authored
      The patch set adding 9p functionality to darwin introduced an issue
      where limits.h, which defines XATTR_SIZE_MAX, is included in 9p.c,
      though the referenced constant is needed in 9p.h. This commit fixes that
      issue by moving the definition of P9_XATTR_SIZE_MAX, which uses
      XATTR_SIZE_MAX, to also be in 9p.c.
      
      Additionally, this commit moves the location of the system headers
      include in 9p.c to occur before the project headers (except osdep.h).
      
      Resolves: https://gitlab.com/qemu-project/qemu/-/issues/950
      
      
      Fixes: 38d7fd68 ("9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX")
      Signed-off-by: default avatarWill Cohen <wwcohen@gmail.com>
      Message-Id: <20220331182651.887-1-wwcohen@gmail.com>
      [thuth: Adjusted placement of osdep.h]
      Signed-off-by: default avatarThomas Huth <thuth@redhat.com>
      a136d175
  7. Mar 31, 2022
  8. Mar 29, 2022
  9. Mar 25, 2022
  10. Mar 24, 2022
  11. Mar 22, 2022
  12. Mar 21, 2022
    • Paolo Bonzini's avatar
      hw/i386/amd_iommu: Fix maybe-uninitialized error with GCC 12 · 17e6ffa6
      Paolo Bonzini authored
      
      Be more explicit that the loop must roll at least once.  Avoids the
      following warning:
      
        FAILED: libqemu-x86_64-softmmu.fa.p/hw_i386_amd_iommu.c.o
        In function 'pte_get_page_mask',
            inlined from 'amdvi_page_walk' at hw/i386/amd_iommu.c:945:25,
            inlined from 'amdvi_do_translate' at hw/i386/amd_iommu.c:989:5,
            inlined from 'amdvi_translate' at hw/i386/amd_iommu.c:1038:5:
        hw/i386/amd_iommu.c:877:38: error: 'oldlevel' may be used uninitialized [-Werror=maybe-uninitialized]
          877 |     return ~((1UL << ((oldlevel * 9) + 3)) - 1);
              |                      ~~~~~~~~~~~~~~~~^~~~
        hw/i386/amd_iommu.c: In function 'amdvi_translate':
        hw/i386/amd_iommu.c:906:41: note: 'oldlevel' was declared here
          906 |     unsigned level, present, pte_perms, oldlevel;
              |                                         ^~~~~~~~
        cc1: all warnings being treated as errors
      
      Having:
      
        $ gcc --version
        gcc (Debian 12-20220313-1) 12.0.1 20220314 (experimental)
      
      Reported-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      17e6ffa6
    • Markus Armbruster's avatar
      Use g_new() & friends where that makes obvious sense · b21e2380
      Markus Armbruster authored
      
      g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
      for two reasons.  One, it catches multiplication overflowing size_t.
      Two, it returns T * rather than void *, which lets the compiler catch
      more type errors.
      
      This commit only touches allocations with size arguments of the form
      sizeof(T).
      
      Patch created mechanically with:
      
          $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
      	     --macro-file scripts/cocci-macro-file.h FILES...
      
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Reviewed-by: default avatarCédric Le Goater <clg@kaod.org>
      Reviewed-by: default avatarAlex Bennée <alex.bennee@linaro.org>
      Acked-by: default avatarDr. David Alan Gilbert <dgilbert@redhat.com>
      Message-Id: <20220315144156.1595462-4-armbru@redhat.com>
      Reviewed-by: default avatarPavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
      b21e2380
    • Markus Armbruster's avatar
      9pfs: Use g_new() & friends where that makes obvious sense · 1366244a
      Markus Armbruster authored
      
      g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
      for two reasons.  One, it catches multiplication overflowing size_t.
      Two, it returns T * rather than void *, which lets the compiler catch
      more type errors.
      
      This commit only touches allocations with size arguments of the form
      sizeof(T).
      
      Initial patch created mechanically with:
      
          $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
      	     --macro-file scripts/cocci-macro-file.h FILES...
      
      This uncovers a typing error:
      
          ../hw/9pfs/9p.c: In function ‘qid_path_fullmap’:
          ../hw/9pfs/9p.c:855:13: error: assignment to ‘QpfEntry *’ from incompatible pointer type ‘QppEntry *’ [-Werror=incompatible-pointer-types]
            855 |         val = g_new0(QppEntry, 1);
      	  |             ^
      
      Harmless, because QppEntry is larger than QpfEntry.  Manually fixed to
      allocate a QpfEntry instead.
      
      Cc: Greg Kurz <groug@kaod.org>
      Cc: Christian Schoenebeck <qemu_oss@crudebyte.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Reviewed-by: default avatarChristian Schoenebeck <qemu_oss@crudebyte.com>
      Reviewed-by: default avatarAlex Bennée <alex.bennee@linaro.org>
      Reviewed-by: default avatarGreg Kurz <groug@kaod.org>
      Message-Id: <20220315144156.1595462-3-armbru@redhat.com>
      1366244a
    • Philippe Mathieu-Daudé's avatar
      hw/sd/sdhci: Prohibit DMA accesses to devices · 799f7f01
      Philippe Mathieu-Daudé authored
      
      The issue reported by OSS-Fuzz produces the following backtrace:
      
        ==447470==ERROR: AddressSanitizer: heap-buffer-overflow
        READ of size 1 at 0x61500002a080 thread T0
            #0 0x71766d47 in sdhci_read_dataport hw/sd/sdhci.c:474:18
            #1 0x7175f139 in sdhci_read hw/sd/sdhci.c:1022:19
            #2 0x721b937b in memory_region_read_accessor softmmu/memory.c:440:11
            #3 0x72171e51 in access_with_adjusted_size softmmu/memory.c:554:18
            #4 0x7216f47c in memory_region_dispatch_read1 softmmu/memory.c:1424:16
            #5 0x7216ebb9 in memory_region_dispatch_read softmmu/memory.c:1452:9
            #6 0x7212db5d in flatview_read_continue softmmu/physmem.c:2879:23
            #7 0x7212f958 in flatview_read softmmu/physmem.c:2921:12
            #8 0x7212f418 in address_space_read_full softmmu/physmem.c:2934:18
            #9 0x721305a9 in address_space_rw softmmu/physmem.c:2962:16
            #10 0x7175a392 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
            #11 0x7175a0ea in dma_memory_rw include/sysemu/dma.h:132:12
            #12 0x71759684 in dma_memory_read include/sysemu/dma.h:152:12
            #13 0x7175518c in sdhci_do_adma hw/sd/sdhci.c:823:27
            #14 0x7174bf69 in sdhci_data_transfer hw/sd/sdhci.c:935:13
            #15 0x7176aaa7 in sdhci_send_command hw/sd/sdhci.c:376:9
            #16 0x717629ee in sdhci_write hw/sd/sdhci.c:1212:9
            #17 0x72172513 in memory_region_write_accessor softmmu/memory.c:492:5
            #18 0x72171e51 in access_with_adjusted_size softmmu/memory.c:554:18
            #19 0x72170766 in memory_region_dispatch_write softmmu/memory.c:1504:16
            #20 0x721419ee in flatview_write_continue softmmu/physmem.c:2812:23
            #21 0x721301eb in flatview_write softmmu/physmem.c:2854:12
            #22 0x7212fca8 in address_space_write softmmu/physmem.c:2950:18
            #23 0x721d9a53 in qtest_process_command softmmu/qtest.c:727:9
      
      A DMA descriptor is previously filled in RAM. An I/O access to the
      device (frames #22 to #16) start the DMA engine (frame #13). The
      engine fetch the descriptor and execute the request, which itself
      accesses the SDHCI I/O registers (frame #1 and #0), triggering a
      re-entrancy issue.
      
      Fix by prohibit transactions from the DMA to devices. The DMA engine
      is thus restricted to memories.
      
      Reported-by: OSS-Fuzz (Issue 36391)
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Reviewed-by: default avatarThomas Huth <thuth@redhat.com>
      Resolves: https://gitlab.com/qemu-project/qemu/-/issues/451
      
      
      Message-Id: <20211215205656.488940-3-philmd@redhat.com>
      Signed-off-by: default avatarThomas Huth <thuth@redhat.com>
      799f7f01
    • Philippe Mathieu-Daudé's avatar
      hw/sd/sdhci: Honor failed DMA transactions · 78e619cb
      Philippe Mathieu-Daudé authored
      
      DMA transactions might fail. The DMA API returns a MemTxResult,
      indicating such failures. Do not ignore it. On failure, raise
      the ADMA error flag and eventually triggering an IRQ (see spec
      chapter 1.13.5: "ADMA2 States").
      
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Reviewed-by: default avatarThomas Huth <thuth@redhat.com>
      Message-Id: <20211215205656.488940-2-philmd@redhat.com>
      Signed-off-by: default avatarThomas Huth <thuth@redhat.com>
      78e619cb
    • Philippe Mathieu-Daudé's avatar
      hw/audio/intel-hda: Restrict DMA engine to memories (not MMIO devices) · 79fa9983
      Philippe Mathieu-Daudé authored
      
      Issue #542 reports a reentrancy problem when the DMA engine accesses
      the HDA controller I/O registers. Fix by restricting the DMA engine
      to memories regions (forbidding MMIO devices such the HDA controller).
      
      Reported-by: OSS-Fuzz (Issue 28435)
      Reported-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@redhat.com>
      Reviewed-by: default avatarThomas Huth <thuth@redhat.com>
      Resolves: https://gitlab.com/qemu-project/qemu/-/issues/542
      
      
      CVE: CVE-2021-3611
      Message-Id: <20211218160912.1591633-3-philmd@redhat.com>
      Signed-off-by: default avatarThomas Huth <thuth@redhat.com>
      79fa9983
    • Philippe Mathieu-Daudé's avatar
      hw/audio/intel-hda: Do not ignore DMA overrun errors · be5a8cf3
      Philippe Mathieu-Daudé authored
      
      Per the "High Definition Audio Specification" manual (rev. 1.0a),
      section "3.3.30 Offset 5Dh: RIRBSTS - RIRB Status":
      
        Response Overrun Interrupt Status (RIRBOIS):
      
        Hardware sets this bit to a 1 when an overrun occurs in the RIRB.
        An interrupt may be generated if the Response Overrun Interrupt
        Control bit is set.
      
        This bit will be set if the RIRB DMA engine is not able to write
        the incoming responses to memory before additional incoming
        responses overrun the internal FIFO.
      
        When hardware detects an overrun, it will drop the responses which
        overrun the buffer and set the RIRBOIS status bit to indicate the
        error condition. Optionally, if the RIRBOIC is set, the hardware
        will also generate an error to alert software to the problem.
      
      QEMU emulates the DMA engine with the stl_le_pci_dma() calls. This
      function returns a MemTxResult indicating whether the DMA access
      was successful.
      Handle any MemTxResult error as "DMA engine is not able to write the
      incoming responses to memory" and raise the Overrun Interrupt flag
      when this case occurs.
      
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@redhat.com>
      Message-Id: <20211218160912.1591633-2-philmd@redhat.com>
      Signed-off-by: default avatarThomas Huth <thuth@redhat.com>
      be5a8cf3
  13. Mar 18, 2022
Loading