[RFC PATCH 0/3] hw/audio/intel-hda: Restrict DMA engine to memories (non-MMIO devices)

[RFC PATCH 0/3] hw/audio/intel-hda: Restrict DMA engine to memories (non-MMIO devices)

[Philippe Mathieu-Daudé]

An attempt to fix the Intel HDA DMA reentrancy problem by
restricting the DMA engine to memories, and reporting invalid
I/O region accesses as overruns.

Based-on: <20211218151053.1545962-1-philmd@redhat.com>
"hw: Have DMA API take MemTxAttrs arg & propagate MemTxResult (part 4)"
https://lore.kernel.org/qemu-devel/20211218151053.1545962-1-philmd@redhat.com/

Philippe Mathieu-Daudé (3):
  hw/audio/intel-hda: Do not ignore DMA overrun errors
  hw/audio/intel-hda: Restrict DMA engine to memories (not MMIO devices)
  tests/qtest/intel-hda-test: Add reproducer for issue #542

 hw/audio/intel-hda.c         | 11 ++++++++---
 tests/qtest/intel-hda-test.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

-- 
2.33.1

[RFC PATCH 1/3] hw/audio/intel-hda: Do not ignore DMA overrun errors

[Philippe Mathieu-Daudé]

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: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/audio/intel-hda.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 2b55d521503..0c1017edbbf 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -350,6 +350,7 @@ static void intel_hda_response(HDACodecDevice *dev, bool solicited, uint32_t res
     IntelHDAState *d = container_of(bus, IntelHDAState, codecs);
     hwaddr addr;
     uint32_t wp, ex;
+    MemTxResult res = MEMTX_OK;
 
     if (d->ics & ICH6_IRS_BUSY) {
         dprint(d, 2, "%s: [irr] response 0x%x, cad 0x%x\n",
@@ -368,8 +369,12 @@ static void intel_hda_response(HDACodecDevice *dev, bool solicited, uint32_t res
     ex = (solicited ? 0 : (1 << 4)) | dev->cad;
     wp = (d->rirb_wp + 1) & 0xff;
     addr = intel_hda_addr(d->rirb_lbase, d->rirb_ubase);
-    stl_le_pci_dma(&d->pci, addr + 8 * wp, response, attrs);
-    stl_le_pci_dma(&d->pci, addr + 8 * wp + 4, ex, attrs);
+    res |= stl_le_pci_dma(&d->pci, addr + 8 * wp, response, attrs);
+    res |= stl_le_pci_dma(&d->pci, addr + 8 * wp + 4, ex, attrs);
+    if (res != MEMTX_OK && (d->rirb_ctl & ICH6_RBCTL_OVERRUN_EN)) {
+        d->rirb_sts |= ICH6_RBSTS_OVERRUN;
+        intel_hda_update_irq(d);
+    }
     d->rirb_wp = wp;
 
     dprint(d, 2, "%s: [wp 0x%x] response 0x%x, extra 0x%x\n",
-- 
2.33.1

[RFC PATCH 2/3] hw/audio/intel-hda: Restrict DMA engine to memories (not MMIO devices)

[Philippe Mathieu-Daudé]

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: Alexander Bulekov <alxndr@bu.edu>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/542
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Likely intel_hda_xfer() and intel_hda_corb_run() should be restricted
too.
---
 hw/audio/intel-hda.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 0c1017edbbf..3aa57d274e6 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -345,7 +345,7 @@ static void intel_hda_corb_run(IntelHDAState *d)
 
 static void intel_hda_response(HDACodecDevice *dev, bool solicited, uint32_t response)
 {
-    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;
-- 
2.33.1

[RFC PATCH 3/3] tests/qtest/intel-hda-test: Add reproducer for issue #542

[Philippe Mathieu-Daudé]

Include the qtest reproducer provided by Alexander Bulekov
in https://gitlab.com/qemu-project/qemu/-/issues/542.
Without the previous commit, we get:

  $ make check-qtest-i386
  ...
  Running test tests/qtest/intel-hda-test
  AddressSanitizer:DEADLYSIGNAL
  =================================================================
  ==1580408==ERROR: AddressSanitizer: stack-overflow on address 0x7ffc3d566fe0
      #0 0x63d297cf in address_space_translate_internal softmmu/physmem.c:356
      #1 0x63d27260 in flatview_do_translate softmmu/physmem.c:499:15
      #2 0x63d27af5 in flatview_translate softmmu/physmem.c:565:15
      #3 0x63d4ce84 in flatview_write softmmu/physmem.c:2850:10
      #4 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
      #5 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
      #6 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
      #7 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
      #8 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
      #9 0x62ae5ec0 in stl_le_dma include/sysemu/dma.h:275:1
      #10 0x62ae5ba2 in stl_le_pci_dma include/hw/pci/pci.h:871:1
      #11 0x62ad59a6 in intel_hda_response hw/audio/intel-hda.c:372:12
      #12 0x62ad2afb in hda_codec_response hw/audio/intel-hda.c:107:5
      #13 0x62aec4e1 in hda_audio_command hw/audio/hda-codec.c:655:5
      #14 0x62ae05d9 in intel_hda_send_command hw/audio/intel-hda.c:307:5
      #15 0x62adff54 in intel_hda_corb_run hw/audio/intel-hda.c:342:9
      #16 0x62adc13b in intel_hda_set_corb_wp hw/audio/intel-hda.c:548:5
      #17 0x62ae5942 in intel_hda_reg_write hw/audio/intel-hda.c:977:9
      #18 0x62ada10a in intel_hda_mmio_write hw/audio/intel-hda.c:1054:5
      #19 0x63d8f383 in memory_region_write_accessor softmmu/memory.c:492:5
      #20 0x63d8ecc1 in access_with_adjusted_size softmmu/memory.c:554:18
      #21 0x63d8d5d6 in memory_region_dispatch_write softmmu/memory.c:1504:16
      #22 0x63d5e85e in flatview_write_continue softmmu/physmem.c:2812:23
      #23 0x63d4d05b in flatview_write softmmu/physmem.c:2854:12
      #24 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
      #25 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
      #26 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
      #27 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
      #28 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
      #29 0x62ae5ec0 in stl_le_dma include/sysemu/dma.h:275:1
      #30 0x62ae5ba2 in stl_le_pci_dma include/hw/pci/pci.h:871:1
      #31 0x62ad59a6 in intel_hda_response hw/audio/intel-hda.c:372:12
      #32 0x62ad2afb in hda_codec_response hw/audio/intel-hda.c:107:5
      #33 0x62aec4e1 in hda_audio_command hw/audio/hda-codec.c:655:5
      #34 0x62ae05d9 in intel_hda_send_command hw/audio/intel-hda.c:307:5
      #35 0x62adff54 in intel_hda_corb_run hw/audio/intel-hda.c:342:9
      #36 0x62adc13b in intel_hda_set_corb_wp hw/audio/intel-hda.c:548:5
      #37 0x62ae5942 in intel_hda_reg_write hw/audio/intel-hda.c:977:9
      #38 0x62ada10a in intel_hda_mmio_write hw/audio/intel-hda.c:1054:5
      #39 0x63d8f383 in memory_region_write_accessor softmmu/memory.c:492:5
      #40 0x63d8ecc1 in access_with_adjusted_size softmmu/memory.c:554:18
      #41 0x63d8d5d6 in memory_region_dispatch_write softmmu/memory.c:1504:16
      #42 0x63d5e85e in flatview_write_continue softmmu/physmem.c:2812:23
      #43 0x63d4d05b in flatview_write softmmu/physmem.c:2854:12
      #44 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
      #45 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
      #46 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
      #47 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
      #48 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
      ...
  SUMMARY: AddressSanitizer: stack-overflow softmmu/physmem.c:356 in address_space_translate_internal
  ==1580408==ABORTING
  Broken pipe
  Aborted (core dumped)

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/intel-hda-test.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
index fc25ccc33cc..a58c98e4d11 100644
--- a/tests/qtest/intel-hda-test.c
+++ b/tests/qtest/intel-hda-test.c
@@ -29,11 +29,45 @@ static void ich9_test(void)
     qtest_end();
 }
 
+/*
+ * https://gitlab.com/qemu-project/qemu/-/issues/542
+ * Used to trigger:
+ *  AddressSanitizer: stack-overflow
+ */
+static void test_issue542_ich6(void)
+{
+    QTestState *s;
+
+    s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
+                   "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
+
+    qtest_outl(s, 0xcf8, 0x80000804);
+    qtest_outw(s, 0xcfc, 0x06);
+    qtest_bufwrite(s, 0xff0d060f, "\x03", 1);
+    qtest_bufwrite(s, 0x0, "\x12", 1);
+    qtest_bufwrite(s, 0x2, "\x2a", 1);
+    qtest_writeb(s, 0x0, 0x12);
+    qtest_writeb(s, 0x2, 0x2a);
+    qtest_outl(s, 0xcf8, 0x80000811);
+    qtest_outl(s, 0xcfc, 0x006a4400);
+    qtest_bufwrite(s, 0x6a44005a, "\x01", 1);
+    qtest_bufwrite(s, 0x6a44005c, "\x02", 1);
+    qtest_bufwrite(s, 0x6a442050, "\x00\x00\x44\x6a", 4);
+    qtest_bufwrite(s, 0x6a44204a, "\x01", 1);
+    qtest_bufwrite(s, 0x6a44204c, "\x02", 1);
+    qtest_bufwrite(s, 0x6a44005c, "\x02", 1);
+    qtest_bufwrite(s, 0x6a442050, "\x00\x00\x44\x6a", 4);
+    qtest_bufwrite(s, 0x6a44204a, "\x01", 1);
+    qtest_bufwrite(s, 0x6a44204c, "\x02", 1);
+    qtest_quit(s);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/intel-hda/ich6", ich6_test);
     qtest_add_func("/intel-hda/ich9", ich9_test);
+    qtest_add_func("/intel-hda/fuzz/issue542", test_issue542_ich6);
 
     return g_test_run();
 }
-- 
2.33.1

Re: [RFC PATCH 0/3] hw/audio/intel-hda: Restrict DMA engine to memories (CVE-2021-3611)

[Philippe Mathieu-Daudé]

On 12/18/21 17:09, Philippe Mathieu-Daudé wrote:
> An attempt to fix the Intel HDA DMA reentrancy problem by
> restricting the DMA engine to memories, and reporting invalid
> I/O region accesses as overruns.

Eh reading again the gitlab issue I realized CVE-2021-3611 has
been assigned for this.

Re: [RFC PATCH 2/3] hw/audio/intel-hda: Restrict DMA engine to memories (not MMIO devices) [CVE-2021-3611]

[Thomas Huth]

On 18/12/2021 17.09, Philippe Mathieu-Daudé wrote:
> 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: Alexander Bulekov <alxndr@bu.edu>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/542
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Likely intel_hda_xfer() and intel_hda_corb_run() should be restricted
> too.
> ---
>   hw/audio/intel-hda.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> index 0c1017edbbf..3aa57d274e6 100644
> --- a/hw/audio/intel-hda.c
> +++ b/hw/audio/intel-hda.c
> @@ -345,7 +345,7 @@ static void intel_hda_corb_run(IntelHDAState *d)
>   
>   static void intel_hda_response(HDACodecDevice *dev, bool solicited, uint32_t response)
>   {
> -    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;

That's maybe the best we can do right now to fix CVE-2021-3611 !

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [RFC PATCH 3/3] tests/qtest/intel-hda-test: Add reproducer for issue #542

[Thomas Huth]

On 18/12/2021 17.09, Philippe Mathieu-Daudé wrote:
> Include the qtest reproducer provided by Alexander Bulekov
> in https://gitlab.com/qemu-project/qemu/-/issues/542.
> Without the previous commit, we get:
> 
>    $ make check-qtest-i386
>    ...
>    Running test tests/qtest/intel-hda-test
>    AddressSanitizer:DEADLYSIGNAL
>    =================================================================
>    ==1580408==ERROR: AddressSanitizer: stack-overflow on address 0x7ffc3d566fe0
>        #0 0x63d297cf in address_space_translate_internal softmmu/physmem.c:356
>        #1 0x63d27260 in flatview_do_translate softmmu/physmem.c:499:15
>        #2 0x63d27af5 in flatview_translate softmmu/physmem.c:565:15
>        #3 0x63d4ce84 in flatview_write softmmu/physmem.c:2850:10
>        #4 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
>        #5 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
>        #6 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
>        #7 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
>        #8 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
>        #9 0x62ae5ec0 in stl_le_dma include/sysemu/dma.h:275:1
>        #10 0x62ae5ba2 in stl_le_pci_dma include/hw/pci/pci.h:871:1
>        #11 0x62ad59a6 in intel_hda_response hw/audio/intel-hda.c:372:12
>        #12 0x62ad2afb in hda_codec_response hw/audio/intel-hda.c:107:5
>        #13 0x62aec4e1 in hda_audio_command hw/audio/hda-codec.c:655:5
>        #14 0x62ae05d9 in intel_hda_send_command hw/audio/intel-hda.c:307:5
>        #15 0x62adff54 in intel_hda_corb_run hw/audio/intel-hda.c:342:9
>        #16 0x62adc13b in intel_hda_set_corb_wp hw/audio/intel-hda.c:548:5
>        #17 0x62ae5942 in intel_hda_reg_write hw/audio/intel-hda.c:977:9
>        #18 0x62ada10a in intel_hda_mmio_write hw/audio/intel-hda.c:1054:5
>        #19 0x63d8f383 in memory_region_write_accessor softmmu/memory.c:492:5
>        #20 0x63d8ecc1 in access_with_adjusted_size softmmu/memory.c:554:18
>        #21 0x63d8d5d6 in memory_region_dispatch_write softmmu/memory.c:1504:16
>        #22 0x63d5e85e in flatview_write_continue softmmu/physmem.c:2812:23
>        #23 0x63d4d05b in flatview_write softmmu/physmem.c:2854:12
>        #24 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
>        #25 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
>        #26 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
>        #27 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
>        #28 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
>        #29 0x62ae5ec0 in stl_le_dma include/sysemu/dma.h:275:1
>        #30 0x62ae5ba2 in stl_le_pci_dma include/hw/pci/pci.h:871:1
>        #31 0x62ad59a6 in intel_hda_response hw/audio/intel-hda.c:372:12
>        #32 0x62ad2afb in hda_codec_response hw/audio/intel-hda.c:107:5
>        #33 0x62aec4e1 in hda_audio_command hw/audio/hda-codec.c:655:5
>        #34 0x62ae05d9 in intel_hda_send_command hw/audio/intel-hda.c:307:5
>        #35 0x62adff54 in intel_hda_corb_run hw/audio/intel-hda.c:342:9
>        #36 0x62adc13b in intel_hda_set_corb_wp hw/audio/intel-hda.c:548:5
>        #37 0x62ae5942 in intel_hda_reg_write hw/audio/intel-hda.c:977:9
>        #38 0x62ada10a in intel_hda_mmio_write hw/audio/intel-hda.c:1054:5
>        #39 0x63d8f383 in memory_region_write_accessor softmmu/memory.c:492:5
>        #40 0x63d8ecc1 in access_with_adjusted_size softmmu/memory.c:554:18
>        #41 0x63d8d5d6 in memory_region_dispatch_write softmmu/memory.c:1504:16
>        #42 0x63d5e85e in flatview_write_continue softmmu/physmem.c:2812:23
>        #43 0x63d4d05b in flatview_write softmmu/physmem.c:2854:12
>        #44 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
>        #45 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
>        #46 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
>        #47 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
>        #48 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
>        ...
>    SUMMARY: AddressSanitizer: stack-overflow softmmu/physmem.c:356 in address_space_translate_internal
>    ==1580408==ABORTING
>    Broken pipe
>    Aborted (core dumped)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   tests/qtest/intel-hda-test.c | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
> index fc25ccc33cc..a58c98e4d11 100644
> --- a/tests/qtest/intel-hda-test.c
> +++ b/tests/qtest/intel-hda-test.c
> @@ -29,11 +29,45 @@ static void ich9_test(void)
>       qtest_end();
>   }
>   
> +/*
> + * https://gitlab.com/qemu-project/qemu/-/issues/542
> + * Used to trigger:
> + *  AddressSanitizer: stack-overflow
> + */
> +static void test_issue542_ich6(void)
> +{
> +    QTestState *s;
> +
> +    s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
> +                   "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
> +
> +    qtest_outl(s, 0xcf8, 0x80000804);
> +    qtest_outw(s, 0xcfc, 0x06);
> +    qtest_bufwrite(s, 0xff0d060f, "\x03", 1);
> +    qtest_bufwrite(s, 0x0, "\x12", 1);
> +    qtest_bufwrite(s, 0x2, "\x2a", 1);
> +    qtest_writeb(s, 0x0, 0x12);
> +    qtest_writeb(s, 0x2, 0x2a);
> +    qtest_outl(s, 0xcf8, 0x80000811);
> +    qtest_outl(s, 0xcfc, 0x006a4400);
> +    qtest_bufwrite(s, 0x6a44005a, "\x01", 1);
> +    qtest_bufwrite(s, 0x6a44005c, "\x02", 1);
> +    qtest_bufwrite(s, 0x6a442050, "\x00\x00\x44\x6a", 4);
> +    qtest_bufwrite(s, 0x6a44204a, "\x01", 1);
> +    qtest_bufwrite(s, 0x6a44204c, "\x02", 1);
> +    qtest_bufwrite(s, 0x6a44005c, "\x02", 1);
> +    qtest_bufwrite(s, 0x6a442050, "\x00\x00\x44\x6a", 4);
> +    qtest_bufwrite(s, 0x6a44204a, "\x01", 1);
> +    qtest_bufwrite(s, 0x6a44204c, "\x02", 1);
> +    qtest_quit(s);
> +}
> +
>   int main(int argc, char **argv)
>   {
>       g_test_init(&argc, &argv, NULL);
>       qtest_add_func("/intel-hda/ich6", ich6_test);
>       qtest_add_func("/intel-hda/ich9", ich9_test);
> +    qtest_add_func("/intel-hda/fuzz/issue542", test_issue542_ich6);
>   
>       return g_test_run();
>   }

Acked-by: Thomas Huth <thuth@redhat.com>