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

jianghaotian.sunday@gmail.com posted 1 patch 1 day, 2 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260723021412.377244-1-jianghaotian.sunday@gmail.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
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 1 day, 2 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é 22 hours ago
On 23/7/26 04:14, 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(-)
> 
> 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));

What difference with commit c257745248b02fd468619b4b1fdbbade5344191d?