[PATCH] hw/audio/intel-hda: restrict all DMA engine paths to memories

jianghaotian.sunday@gmail.com posted 1 patch 4 days, 3 hours ago
Failed in applying to current master (apply log)
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
hw/audio/intel-hda.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
[PATCH] hw/audio/intel-hda: restrict all DMA engine paths to memories
Posted by jianghaotian.sunday@gmail.com 4 days, 3 hours ago
From: Haotian Jiang <jianghaotian.sunday@gmail.com>

CVE-2021-3611 (commit 79fa99831d) restricted the DMA engine to memories
by setting attrs.memory=true, but only applied this to intel_hda_response.
Three other DMA engine access points still use MEMTXATTRS_UNSPECIFIED,
allowing a malicious guest to trigger DMA-to-self-MMIO reentry:

  - intel_hda_xfer (line 398): called from the audio timer callback
    (hda_codec_xfer -> bus->xfer), so the MemReentrancyGuard does not
    fire (engaged_in_io is false outside MMIO dispatch). A guest that
    points a BDL entry at the HDA controller's own MMIO BAR can write
    audio samples to device registers, triggering whandler side effects
    such as starting/stopping streams or injecting codec commands via
    CORBWP.
  - intel_hda_parse_bdl (line 478): uses pci_dma_read which hardcodes
    MEMTXATTRS_UNSPECIFIED. A guest-controlled BDL base address can
    point at controller MMIO, allowing the DMA engine to read device
    registers as BDL descriptors.
  - intel_hda_corb_run (line 333): ldl_le_pci_dma reads the CORB ring
    with MEMTXATTRS_UNSPECIFIED, allowing the DMA engine to read
    controller MMIO as CORB entries.

Fix all three by passing {.memory = true} explicitly, matching the
fix already applied to intel_hda_response. For intel_hda_parse_bdl,
replace pci_dma_read with pci_dma_rw to pass the controlled attrs.

Fixes: 79fa99831d ("hw/audio/intel-hda: Restrict DMA engine to memories (not MMIO devices)")
Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
Cc: qemu-stable@nongnu.org
---
 hw/audio/intel-hda.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index d7c2c3c2fd..3d361a4976 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -305,6 +305,7 @@ static int intel_hda_send_command(IntelHDAState *d, uint32_t verb)
 
 static void intel_hda_corb_run(IntelHDAState *d)
 {
+    const MemTxAttrs attrs = { .memory = true };
     hwaddr addr;
     uint32_t rp, verb;
 
@@ -330,7 +331,7 @@ static void intel_hda_corb_run(IntelHDAState *d)
 
         rp = (d->corb_rp + 1) & 0xff;
         addr = intel_hda_addr(d->corb_lbase, d->corb_ubase);
-        ldl_le_pci_dma(&d->pci, addr + 4 * rp, &verb, MEMTXATTRS_UNSPECIFIED);
+        ldl_le_pci_dma(&d->pci, addr + 4 * rp, &verb, attrs);
         d->corb_rp = rp;
 
         dprint(d, 2, "%s: [rp 0x%x] verb 0x%08x\n", __func__, rp, verb);
@@ -395,7 +396,7 @@ static void intel_hda_response(HDACodecDevice *dev, bool solicited, uint32_t res
 static bool intel_hda_xfer(HDACodecDevice *dev, uint32_t stnr, bool output,
                            uint8_t *buf, uint32_t len)
 {
-    const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
+    const MemTxAttrs attrs = { .memory = true };
     HDACodecBus *bus = HDA_BUS(dev->qdev.parent_bus);
     IntelHDAState *d = container_of(bus, IntelHDAState, codecs);
     hwaddr addr;
@@ -466,6 +467,7 @@ static bool intel_hda_xfer(HDACodecDevice *dev, uint32_t stnr, bool output,
 
 static void intel_hda_parse_bdl(IntelHDAState *d, IntelHDAStream *st)
 {
+    const MemTxAttrs attrs = { .memory = true };
     hwaddr addr;
     uint8_t buf[16];
     uint32_t i;
@@ -475,7 +477,8 @@ static void intel_hda_parse_bdl(IntelHDAState *d, IntelHDAStream *st)
     g_free(st->bpl);
     st->bpl = g_new(bpl, st->bentries);
     for (i = 0; i < st->bentries; i++, addr += 16) {
-        pci_dma_read(&d->pci, addr, buf, 16);
+        pci_dma_rw(&d->pci, addr, buf, 16,
+                   DMA_DIRECTION_TO_DEVICE, attrs);
         st->bpl[i].addr  = le64_to_cpu(*(uint64_t *)buf);
         st->bpl[i].len   = le32_to_cpu(*(uint32_t *)(buf + 8));
         st->bpl[i].flags = le32_to_cpu(*(uint32_t *)(buf + 12));
-- 
2.34.1
Re: [PATCH] hw/audio/intel-hda: restrict all DMA engine paths to memories
Posted by Philippe Mathieu-Daudé 3 days, 23 hours ago
On 21/7/26 08:09, jianghaotian.sunday@gmail.com wrote:
> From: Haotian Jiang <jianghaotian.sunday@gmail.com>
> 
> CVE-2021-3611 (commit 79fa99831d) restricted the DMA engine to memories
> by setting attrs.memory=true, but only applied this to intel_hda_response.
> Three other DMA engine access points still use MEMTXATTRS_UNSPECIFIED,
> allowing a malicious guest to trigger DMA-to-self-MMIO reentry:
> 
>    - intel_hda_xfer (line 398): called from the audio timer callback
>      (hda_codec_xfer -> bus->xfer), so the MemReentrancyGuard does not
>      fire (engaged_in_io is false outside MMIO dispatch). A guest that
>      points a BDL entry at the HDA controller's own MMIO BAR can write
>      audio samples to device registers, triggering whandler side effects
>      such as starting/stopping streams or injecting codec commands via
>      CORBWP.
>    - intel_hda_parse_bdl (line 478): uses pci_dma_read which hardcodes
>      MEMTXATTRS_UNSPECIFIED. A guest-controlled BDL base address can
>      point at controller MMIO, allowing the DMA engine to read device
>      registers as BDL descriptors.
>    - intel_hda_corb_run (line 333): ldl_le_pci_dma reads the CORB ring
>      with MEMTXATTRS_UNSPECIFIED, allowing the DMA engine to read
>      controller MMIO as CORB entries.
> 
> Fix all three by passing {.memory = true} explicitly, matching the
> fix already applied to intel_hda_response. For intel_hda_parse_bdl,
> replace pci_dma_read with pci_dma_rw to pass the controlled attrs.
> 
> Fixes: 79fa99831d ("hw/audio/intel-hda: Restrict DMA engine to memories (not MMIO devices)")
> Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
> Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
> Cc: qemu-stable@nongnu.org
> ---
>   hw/audio/intel-hda.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)

Patch queued, thanks.
Re: [PATCH] hw/audio/intel-hda: restrict all DMA engine paths to memories
Posted by Philippe Mathieu-Daudé 3 days, 23 hours ago
On 21/7/26 08:09, jianghaotian.sunday@gmail.com wrote:
> From: Haotian Jiang <jianghaotian.sunday@gmail.com>
> 
> CVE-2021-3611 (commit 79fa99831d) restricted the DMA engine to memories
> by setting attrs.memory=true, but only applied this to intel_hda_response.
> Three other DMA engine access points still use MEMTXATTRS_UNSPECIFIED,
> allowing a malicious guest to trigger DMA-to-self-MMIO reentry:
> 
>    - intel_hda_xfer (line 398): called from the audio timer callback
>      (hda_codec_xfer -> bus->xfer), so the MemReentrancyGuard does not
>      fire (engaged_in_io is false outside MMIO dispatch). A guest that
>      points a BDL entry at the HDA controller's own MMIO BAR can write
>      audio samples to device registers, triggering whandler side effects
>      such as starting/stopping streams or injecting codec commands via
>      CORBWP.
>    - intel_hda_parse_bdl (line 478): uses pci_dma_read which hardcodes
>      MEMTXATTRS_UNSPECIFIED. A guest-controlled BDL base address can
>      point at controller MMIO, allowing the DMA engine to read device
>      registers as BDL descriptors.
>    - intel_hda_corb_run (line 333): ldl_le_pci_dma reads the CORB ring
>      with MEMTXATTRS_UNSPECIFIED, allowing the DMA engine to read
>      controller MMIO as CORB entries.
> 
> Fix all three by passing {.memory = true} explicitly, matching the
> fix already applied to intel_hda_response. For intel_hda_parse_bdl,
> replace pci_dma_read with pci_dma_rw to pass the controlled attrs.
> 
> Fixes: 79fa99831d ("hw/audio/intel-hda: Restrict DMA engine to memories (not MMIO devices)")
> Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
> Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
> Cc: qemu-stable@nongnu.org
> ---
>   hw/audio/intel-hda.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>