[PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)

[PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)

[Mauro Matteo Cascella]

This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
leading to memory corruption bugs like stack overflow or use-after-free.

Fixes: CVE-2023-0330
Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
Reported-by: Zheyu Ma <zheyuma97@gmail.com>
---
 hw/scsi/lsi53c895a.c               | 14 +++++++++----
 tests/qtest/fuzz-lsi53c895a-test.c | 32 ++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index af93557a9a..89c52594eb 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
 static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
                                void *buf, dma_addr_t len)
 {
+    const MemTxAttrs attrs = { .memory = true };
+
     if (s->dmode & LSI_DMODE_SIOM) {
-        address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
+        address_space_read(&s->pci_io_as, addr, attrs,
                            buf, len);
     } else {
-        pci_dma_read(PCI_DEVICE(s), addr, buf, len);
+        pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
+                      DMA_DIRECTION_TO_DEVICE, attrs);
     }
 }
 
 static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
                                 const void *buf, dma_addr_t len)
 {
+    const MemTxAttrs attrs = { .memory = true };
+
     if (s->dmode & LSI_DMODE_DIOM) {
-        address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
+        address_space_write(&s->pci_io_as, addr, attrs,
                             buf, len);
     } else {
-        pci_dma_write(PCI_DEVICE(s), addr, buf, len);
+        pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
+                      DMA_DIRECTION_FROM_DEVICE, attrs);
     }
 }
 
diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c
index 392a7ae7ed..35c02e89f3 100644
--- a/tests/qtest/fuzz-lsi53c895a-test.c
+++ b/tests/qtest/fuzz-lsi53c895a-test.c
@@ -8,6 +8,35 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 
+/*
+ * This used to trigger a DMA reentrancy issue
+ * leading to memory corruption bugs like stack
+ * overflow or use-after-free
+ */
+static void test_lsi_dma_reentrancy(void)
+{
+    QTestState *s;
+
+    s = qtest_init("-M q35 -m 512M -nodefaults "
+                   "-blockdev driver=null-co,node-name=null0 "
+                   "-device lsi53c810 -device scsi-cd,drive=null0");
+
+    qtest_outl(s, 0xcf8, 0x80000804); /* PCI Command Register */
+    qtest_outw(s, 0xcfc, 0x7);        /* Enables accesses */
+    qtest_outl(s, 0xcf8, 0x80000814); /* Memory Bar 1 */
+    qtest_outl(s, 0xcfc, 0xff100000); /* Set MMIO Address*/
+    qtest_outl(s, 0xcf8, 0x80000818); /* Memory Bar 2 */
+    qtest_outl(s, 0xcfc, 0xff000000); /* Set RAM Address*/
+    qtest_writel(s, 0xff000000, 0xc0000024);
+    qtest_writel(s, 0xff000114, 0x00000080);
+    qtest_writel(s, 0xff00012c, 0xff000000);
+    qtest_writel(s, 0xff000004, 0xff000114);
+    qtest_writel(s, 0xff000008, 0xff100014);
+    qtest_writel(s, 0xff10002f, 0x000000ff);
+
+    qtest_quit(s);
+}
+
 /*
  * This used to trigger a UAF in lsi_do_msgout()
  * https://gitlab.com/qemu-project/qemu/-/issues/972
@@ -120,5 +149,8 @@ int main(int argc, char **argv)
     qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
                    test_lsi_do_msgout_cancel_req);
 
+    qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
+                   test_lsi_dma_reentrancy);
+
     return g_test_run();
 }
-- 
2.39.0

Re: [PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)

[Mauro Matteo Cascella]

On Mon, Jan 16, 2023 at 9:42 PM Mauro Matteo Cascella
<mcascell@redhat.com> wrote:
>
> This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
> leading to memory corruption bugs like stack overflow or use-after-free.
>
> Fixes: CVE-2023-0330
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> ---
>  hw/scsi/lsi53c895a.c               | 14 +++++++++----
>  tests/qtest/fuzz-lsi53c895a-test.c | 32 ++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index af93557a9a..89c52594eb 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
>  static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
>                                 void *buf, dma_addr_t len)
>  {
> +    const MemTxAttrs attrs = { .memory = true };
> +
>      if (s->dmode & LSI_DMODE_SIOM) {
> -        address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> +        address_space_read(&s->pci_io_as, addr, attrs,
>                             buf, len);
>      } else {
> -        pci_dma_read(PCI_DEVICE(s), addr, buf, len);
> +        pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
> +                      DMA_DIRECTION_TO_DEVICE, attrs);
>      }
>  }
>
>  static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
>                                  const void *buf, dma_addr_t len)
>  {
> +    const MemTxAttrs attrs = { .memory = true };
> +
>      if (s->dmode & LSI_DMODE_DIOM) {
> -        address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> +        address_space_write(&s->pci_io_as, addr, attrs,
>                              buf, len);
>      } else {
> -        pci_dma_write(PCI_DEVICE(s), addr, buf, len);
> +        pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
> +                      DMA_DIRECTION_FROM_DEVICE, attrs);
>      }
>  }
>
> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c
> index 392a7ae7ed..35c02e89f3 100644
> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> @@ -8,6 +8,35 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>
> +/*
> + * This used to trigger a DMA reentrancy issue
> + * leading to memory corruption bugs like stack
> + * overflow or use-after-free
> + */
> +static void test_lsi_dma_reentrancy(void)
> +{
> +    QTestState *s;
> +
> +    s = qtest_init("-M q35 -m 512M -nodefaults "
> +                   "-blockdev driver=null-co,node-name=null0 "
> +                   "-device lsi53c810 -device scsi-cd,drive=null0");
> +
> +    qtest_outl(s, 0xcf8, 0x80000804); /* PCI Command Register */
> +    qtest_outw(s, 0xcfc, 0x7);        /* Enables accesses */
> +    qtest_outl(s, 0xcf8, 0x80000814); /* Memory Bar 1 */
> +    qtest_outl(s, 0xcfc, 0xff100000); /* Set MMIO Address*/
> +    qtest_outl(s, 0xcf8, 0x80000818); /* Memory Bar 2 */
> +    qtest_outl(s, 0xcfc, 0xff000000); /* Set RAM Address*/
> +    qtest_writel(s, 0xff000000, 0xc0000024);
> +    qtest_writel(s, 0xff000114, 0x00000080);
> +    qtest_writel(s, 0xff00012c, 0xff000000);
> +    qtest_writel(s, 0xff000004, 0xff000114);
> +    qtest_writel(s, 0xff000008, 0xff100014);
> +    qtest_writel(s, 0xff10002f, 0x000000ff);
> +
> +    qtest_quit(s);
> +}
> +
>  /*
>   * This used to trigger a UAF in lsi_do_msgout()
>   * https://gitlab.com/qemu-project/qemu/-/issues/972
> @@ -120,5 +149,8 @@ int main(int argc, char **argv)
>      qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
>                     test_lsi_do_msgout_cancel_req);
>
> +    qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
> +                   test_lsi_dma_reentrancy);
> +
>      return g_test_run();
>  }
> --
> 2.39.0
>

Reproducer:

cat << EOF | ./x86_64-softmmu/qemu-system-x86_64 -machine accel=qtest \
-m 512M -machine q35 -nodefaults -device lsi53c810 -device scsi-cd,drive=null0 \
-display none -blockdev driver=null-co,node-name=null0 -qtest stdio
outl 0xcf8 0x80000804   /* PCI Command Register */
outl 0xcfc 0x7                 /* Enable accesses */
outl 0xcf8 0x80000814   /* Memory Bar 1 */
outl 0xcfc 0xff100000     /* Set MMIO Address*/
outl 0xcf8 0x80000818   /* Memory Bar 2 */
outl 0xcfc 0xff000000     /* Set RAM Address*/
writel 0xff000000 0xc0000024
writel 0xff000114 0x00000080
writel 0xff00012c 0xff000000
writel 0xff000004 0xff000114
writel 0xff000008 0xff100014
writel 0xff10002f 0x000000ff
EOF

-- 
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0

Re: [PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)

[Karl Heubaum]

Did this CVE fix fall in the cracks during the QEMU 8.0 merge window?

Karl

> On Jan 16, 2023, at 2:42 PM, Mauro Matteo Cascella <mcascell@redhat.com> wrote:
> 
> This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
> leading to memory corruption bugs like stack overflow or use-after-free.
> 
> Fixes: CVE-2023-0330
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> ---
> hw/scsi/lsi53c895a.c               | 14 +++++++++----
> tests/qtest/fuzz-lsi53c895a-test.c | 32 ++++++++++++++++++++++++++++++
> 2 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index af93557a9a..89c52594eb 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
> static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
>                                void *buf, dma_addr_t len)
> {
> +    const MemTxAttrs attrs = { .memory = true };
> +
>     if (s->dmode & LSI_DMODE_SIOM) {
> -        address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> +        address_space_read(&s->pci_io_as, addr, attrs,
>                            buf, len);
>     } else {
> -        pci_dma_read(PCI_DEVICE(s), addr, buf, len);
> +        pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
> +                      DMA_DIRECTION_TO_DEVICE, attrs);
>     }
> }
> 
> static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
>                                 const void *buf, dma_addr_t len)
> {
> +    const MemTxAttrs attrs = { .memory = true };
> +
>     if (s->dmode & LSI_DMODE_DIOM) {
> -        address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> +        address_space_write(&s->pci_io_as, addr, attrs,
>                             buf, len);
>     } else {
> -        pci_dma_write(PCI_DEVICE(s), addr, buf, len);
> +        pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
> +                      DMA_DIRECTION_FROM_DEVICE, attrs);
>     }
> }
> 
> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c
> index 392a7ae7ed..35c02e89f3 100644
> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> @@ -8,6 +8,35 @@
> #include "qemu/osdep.h"
> #include "libqtest.h"
> 
> +/*
> + * This used to trigger a DMA reentrancy issue
> + * leading to memory corruption bugs like stack
> + * overflow or use-after-free
> + */
> +static void test_lsi_dma_reentrancy(void)
> +{
> +    QTestState *s;
> +
> +    s = qtest_init("-M q35 -m 512M -nodefaults "
> +                   "-blockdev driver=null-co,node-name=null0 "
> +                   "-device lsi53c810 -device scsi-cd,drive=null0");
> +
> +    qtest_outl(s, 0xcf8, 0x80000804); /* PCI Command Register */
> +    qtest_outw(s, 0xcfc, 0x7);        /* Enables accesses */
> +    qtest_outl(s, 0xcf8, 0x80000814); /* Memory Bar 1 */
> +    qtest_outl(s, 0xcfc, 0xff100000); /* Set MMIO Address*/
> +    qtest_outl(s, 0xcf8, 0x80000818); /* Memory Bar 2 */
> +    qtest_outl(s, 0xcfc, 0xff000000); /* Set RAM Address*/
> +    qtest_writel(s, 0xff000000, 0xc0000024);
> +    qtest_writel(s, 0xff000114, 0x00000080);
> +    qtest_writel(s, 0xff00012c, 0xff000000);
> +    qtest_writel(s, 0xff000004, 0xff000114);
> +    qtest_writel(s, 0xff000008, 0xff100014);
> +    qtest_writel(s, 0xff10002f, 0x000000ff);
> +
> +    qtest_quit(s);
> +}
> +
> /*
>  * This used to trigger a UAF in lsi_do_msgout()
>  * https://gitlab.com/qemu-project/qemu/-/issues/972
> @@ -120,5 +149,8 @@ int main(int argc, char **argv)
>     qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
>                    test_lsi_do_msgout_cancel_req);
> 
> +    qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
> +                   test_lsi_dma_reentrancy);
> +
>     return g_test_run();
> }
> -- 
> 2.39.0
> 
> 

Re: [PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)

[Philippe Mathieu-Daudé]

On 17/3/23 19:18, Karl Heubaum wrote:
> Did this CVE fix fall in the cracks during the QEMU 8.0 merge window?

The patch isn't reviewed, and apparently almost no active contributor
understand this device enough to be sure this security patch doesn't
break normal use. Fuzzed bugs are rarely trivial.

>> On Jan 16, 2023, at 2:42 PM, Mauro Matteo Cascella <mcascell@redhat.com> wrote:
>>
>> This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
>> leading to memory corruption bugs like stack overflow or use-after-free.
>>
>> Fixes: CVE-2023-0330
>> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
>> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
>> ---
>> hw/scsi/lsi53c895a.c               | 14 +++++++++----
>> tests/qtest/fuzz-lsi53c895a-test.c | 32 ++++++++++++++++++++++++++++++
>> 2 files changed, 42 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
>> index af93557a9a..89c52594eb 100644
>> --- a/hw/scsi/lsi53c895a.c
>> +++ b/hw/scsi/lsi53c895a.c
>> @@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
>> static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
>>                                 void *buf, dma_addr_t len)
>> {
>> +    const MemTxAttrs attrs = { .memory = true };
>> +
>>      if (s->dmode & LSI_DMODE_SIOM) {
>> -        address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
>> +        address_space_read(&s->pci_io_as, addr, attrs,
>>                             buf, len);
>>      } else {
>> -        pci_dma_read(PCI_DEVICE(s), addr, buf, len);
>> +        pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
>> +                      DMA_DIRECTION_TO_DEVICE, attrs);
>>      }
>> }
>>
>> static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
>>                                  const void *buf, dma_addr_t len)
>> {
>> +    const MemTxAttrs attrs = { .memory = true };
>> +
>>      if (s->dmode & LSI_DMODE_DIOM) {
>> -        address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
>> +        address_space_write(&s->pci_io_as, addr, attrs,
>>                              buf, len);
>>      } else {
>> -        pci_dma_write(PCI_DEVICE(s), addr, buf, len);
>> +        pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
>> +                      DMA_DIRECTION_FROM_DEVICE, attrs);
>>      }
>> }
>>
>> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c
>> index 392a7ae7ed..35c02e89f3 100644
>> --- a/tests/qtest/fuzz-lsi53c895a-test.c
>> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
>> @@ -8,6 +8,35 @@
>> #include "qemu/osdep.h"
>> #include "libqtest.h"
>>
>> +/*
>> + * This used to trigger a DMA reentrancy issue
>> + * leading to memory corruption bugs like stack
>> + * overflow or use-after-free
>> + */
>> +static void test_lsi_dma_reentrancy(void)
>> +{
>> +    QTestState *s;
>> +
>> +    s = qtest_init("-M q35 -m 512M -nodefaults "
>> +                   "-blockdev driver=null-co,node-name=null0 "
>> +                   "-device lsi53c810 -device scsi-cd,drive=null0");
>> +
>> +    qtest_outl(s, 0xcf8, 0x80000804); /* PCI Command Register */
>> +    qtest_outw(s, 0xcfc, 0x7);        /* Enables accesses */
>> +    qtest_outl(s, 0xcf8, 0x80000814); /* Memory Bar 1 */
>> +    qtest_outl(s, 0xcfc, 0xff100000); /* Set MMIO Address*/
>> +    qtest_outl(s, 0xcf8, 0x80000818); /* Memory Bar 2 */
>> +    qtest_outl(s, 0xcfc, 0xff000000); /* Set RAM Address*/
>> +    qtest_writel(s, 0xff000000, 0xc0000024);
>> +    qtest_writel(s, 0xff000114, 0x00000080);
>> +    qtest_writel(s, 0xff00012c, 0xff000000);
>> +    qtest_writel(s, 0xff000004, 0xff000114);
>> +    qtest_writel(s, 0xff000008, 0xff100014);
>> +    qtest_writel(s, 0xff10002f, 0x000000ff);
>> +
>> +    qtest_quit(s);
>> +}
>> +
>> /*
>>   * This used to trigger a UAF in lsi_do_msgout()
>>   * https://gitlab.com/qemu-project/qemu/-/issues/972
>> @@ -120,5 +149,8 @@ int main(int argc, char **argv)
>>      qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
>>                     test_lsi_do_msgout_cancel_req);
>>
>> +    qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
>> +                   test_lsi_dma_reentrancy);
>> +
>>      return g_test_run();
>> }
>> -- 
>> 2.39.0
>>
>>
> 

Re: [PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)

[Mauro Matteo Cascella]

On Fri, Mar 17, 2023 at 10:59 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 17/3/23 19:18, Karl Heubaum wrote:
> > Did this CVE fix fall in the cracks during the QEMU 8.0 merge window?
>
> The patch isn't reviewed, and apparently almost no active contributor
> understand this device enough to be sure this security patch doesn't
> break normal use. Fuzzed bugs are rarely trivial.

I guess Alexander's new patchset [1] does not help fix this one?

[1] https://patchew.org/QEMU/20230313082417.827484-1-alxndr@bu.edu/20230313082417.827484-7-alxndr@bu.edu/


> >> On Jan 16, 2023, at 2:42 PM, Mauro Matteo Cascella <mcascell@redhat.com> wrote:
> >>
> >> This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
> >> leading to memory corruption bugs like stack overflow or use-after-free.
> >>
> >> Fixes: CVE-2023-0330
> >> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> >> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> >> ---
> >> hw/scsi/lsi53c895a.c               | 14 +++++++++----
> >> tests/qtest/fuzz-lsi53c895a-test.c | 32 ++++++++++++++++++++++++++++++
> >> 2 files changed, 42 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> >> index af93557a9a..89c52594eb 100644
> >> --- a/hw/scsi/lsi53c895a.c
> >> +++ b/hw/scsi/lsi53c895a.c
> >> @@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
> >> static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
> >>                                 void *buf, dma_addr_t len)
> >> {
> >> +    const MemTxAttrs attrs = { .memory = true };
> >> +
> >>      if (s->dmode & LSI_DMODE_SIOM) {
> >> -        address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> >> +        address_space_read(&s->pci_io_as, addr, attrs,
> >>                             buf, len);
> >>      } else {
> >> -        pci_dma_read(PCI_DEVICE(s), addr, buf, len);
> >> +        pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
> >> +                      DMA_DIRECTION_TO_DEVICE, attrs);
> >>      }
> >> }
> >>
> >> static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
> >>                                  const void *buf, dma_addr_t len)
> >> {
> >> +    const MemTxAttrs attrs = { .memory = true };
> >> +
> >>      if (s->dmode & LSI_DMODE_DIOM) {
> >> -        address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> >> +        address_space_write(&s->pci_io_as, addr, attrs,
> >>                              buf, len);
> >>      } else {
> >> -        pci_dma_write(PCI_DEVICE(s), addr, buf, len);
> >> +        pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
> >> +                      DMA_DIRECTION_FROM_DEVICE, attrs);
> >>      }
> >> }
> >>
> >> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c
> >> index 392a7ae7ed..35c02e89f3 100644
> >> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> >> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> >> @@ -8,6 +8,35 @@
> >> #include "qemu/osdep.h"
> >> #include "libqtest.h"
> >>
> >> +/*
> >> + * This used to trigger a DMA reentrancy issue
> >> + * leading to memory corruption bugs like stack
> >> + * overflow or use-after-free
> >> + */
> >> +static void test_lsi_dma_reentrancy(void)
> >> +{
> >> +    QTestState *s;
> >> +
> >> +    s = qtest_init("-M q35 -m 512M -nodefaults "
> >> +                   "-blockdev driver=null-co,node-name=null0 "
> >> +                   "-device lsi53c810 -device scsi-cd,drive=null0");
> >> +
> >> +    qtest_outl(s, 0xcf8, 0x80000804); /* PCI Command Register */
> >> +    qtest_outw(s, 0xcfc, 0x7);        /* Enables accesses */
> >> +    qtest_outl(s, 0xcf8, 0x80000814); /* Memory Bar 1 */
> >> +    qtest_outl(s, 0xcfc, 0xff100000); /* Set MMIO Address*/
> >> +    qtest_outl(s, 0xcf8, 0x80000818); /* Memory Bar 2 */
> >> +    qtest_outl(s, 0xcfc, 0xff000000); /* Set RAM Address*/
> >> +    qtest_writel(s, 0xff000000, 0xc0000024);
> >> +    qtest_writel(s, 0xff000114, 0x00000080);
> >> +    qtest_writel(s, 0xff00012c, 0xff000000);
> >> +    qtest_writel(s, 0xff000004, 0xff000114);
> >> +    qtest_writel(s, 0xff000008, 0xff100014);
> >> +    qtest_writel(s, 0xff10002f, 0x000000ff);
> >> +
> >> +    qtest_quit(s);
> >> +}
> >> +
> >> /*
> >>   * This used to trigger a UAF in lsi_do_msgout()
> >>   * https://gitlab.com/qemu-project/qemu/-/issues/972
> >> @@ -120,5 +149,8 @@ int main(int argc, char **argv)
> >>      qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
> >>                     test_lsi_do_msgout_cancel_req);
> >>
> >> +    qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
> >> +                   test_lsi_dma_reentrancy);
> >> +
> >>      return g_test_run();
> >> }
> >> --
> >> 2.39.0
> >>
> >>
> >
>


--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0

Re: [PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)

[Alexander Bulekov]

On 230324 1200, Mauro Matteo Cascella wrote:
> On Fri, Mar 17, 2023 at 10:59 PM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
> >
> > On 17/3/23 19:18, Karl Heubaum wrote:
> > > Did this CVE fix fall in the cracks during the QEMU 8.0 merge window?
> >
> > The patch isn't reviewed, and apparently almost no active contributor
> > understand this device enough to be sure this security patch doesn't
> > break normal use. Fuzzed bugs are rarely trivial.
> 
> I guess Alexander's new patchset [1] does not help fix this one?
> 
> [1] https://patchew.org/QEMU/20230313082417.827484-1-alxndr@bu.edu/20230313082417.827484-7-alxndr@bu.edu/
> 

It should cover it. At least the reproducer in this email no longer
works.
-Alex

> 
> > >> On Jan 16, 2023, at 2:42 PM, Mauro Matteo Cascella <mcascell@redhat.com> wrote:
> > >>
> > >> This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
> > >> leading to memory corruption bugs like stack overflow or use-after-free.
> > >>
> > >> Fixes: CVE-2023-0330
> > >> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> > >> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> > >> ---
> > >> hw/scsi/lsi53c895a.c               | 14 +++++++++----
> > >> tests/qtest/fuzz-lsi53c895a-test.c | 32 ++++++++++++++++++++++++++++++
> > >> 2 files changed, 42 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> > >> index af93557a9a..89c52594eb 100644
> > >> --- a/hw/scsi/lsi53c895a.c
> > >> +++ b/hw/scsi/lsi53c895a.c
> > >> @@ -446,22 +446,28 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
> > >> static inline void lsi_mem_read(LSIState *s, dma_addr_t addr,
> > >>                                 void *buf, dma_addr_t len)
> > >> {
> > >> +    const MemTxAttrs attrs = { .memory = true };
> > >> +
> > >>      if (s->dmode & LSI_DMODE_SIOM) {
> > >> -        address_space_read(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> > >> +        address_space_read(&s->pci_io_as, addr, attrs,
> > >>                             buf, len);
> > >>      } else {
> > >> -        pci_dma_read(PCI_DEVICE(s), addr, buf, len);
> > >> +        pci_dma_rw(PCI_DEVICE(s), addr, buf, len,
> > >> +                      DMA_DIRECTION_TO_DEVICE, attrs);
> > >>      }
> > >> }
> > >>
> > >> static inline void lsi_mem_write(LSIState *s, dma_addr_t addr,
> > >>                                  const void *buf, dma_addr_t len)
> > >> {
> > >> +    const MemTxAttrs attrs = { .memory = true };
> > >> +
> > >>      if (s->dmode & LSI_DMODE_DIOM) {
> > >> -        address_space_write(&s->pci_io_as, addr, MEMTXATTRS_UNSPECIFIED,
> > >> +        address_space_write(&s->pci_io_as, addr, attrs,
> > >>                              buf, len);
> > >>      } else {
> > >> -        pci_dma_write(PCI_DEVICE(s), addr, buf, len);
> > >> +        pci_dma_rw(PCI_DEVICE(s), addr, (void *) buf, len,
> > >> +                      DMA_DIRECTION_FROM_DEVICE, attrs);
> > >>      }
> > >> }
> > >>
> > >> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c
> > >> index 392a7ae7ed..35c02e89f3 100644
> > >> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> > >> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> > >> @@ -8,6 +8,35 @@
> > >> #include "qemu/osdep.h"
> > >> #include "libqtest.h"
> > >>
> > >> +/*
> > >> + * This used to trigger a DMA reentrancy issue
> > >> + * leading to memory corruption bugs like stack
> > >> + * overflow or use-after-free
> > >> + */
> > >> +static void test_lsi_dma_reentrancy(void)
> > >> +{
> > >> +    QTestState *s;
> > >> +
> > >> +    s = qtest_init("-M q35 -m 512M -nodefaults "
> > >> +                   "-blockdev driver=null-co,node-name=null0 "
> > >> +                   "-device lsi53c810 -device scsi-cd,drive=null0");
> > >> +
> > >> +    qtest_outl(s, 0xcf8, 0x80000804); /* PCI Command Register */
> > >> +    qtest_outw(s, 0xcfc, 0x7);        /* Enables accesses */
> > >> +    qtest_outl(s, 0xcf8, 0x80000814); /* Memory Bar 1 */
> > >> +    qtest_outl(s, 0xcfc, 0xff100000); /* Set MMIO Address*/
> > >> +    qtest_outl(s, 0xcf8, 0x80000818); /* Memory Bar 2 */
> > >> +    qtest_outl(s, 0xcfc, 0xff000000); /* Set RAM Address*/
> > >> +    qtest_writel(s, 0xff000000, 0xc0000024);
> > >> +    qtest_writel(s, 0xff000114, 0x00000080);
> > >> +    qtest_writel(s, 0xff00012c, 0xff000000);
> > >> +    qtest_writel(s, 0xff000004, 0xff000114);
> > >> +    qtest_writel(s, 0xff000008, 0xff100014);
> > >> +    qtest_writel(s, 0xff10002f, 0x000000ff);
> > >> +
> > >> +    qtest_quit(s);
> > >> +}
> > >> +
> > >> /*
> > >>   * This used to trigger a UAF in lsi_do_msgout()
> > >>   * https://gitlab.com/qemu-project/qemu/-/issues/972
> > >> @@ -120,5 +149,8 @@ int main(int argc, char **argv)
> > >>      qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
> > >>                     test_lsi_do_msgout_cancel_req);
> > >>
> > >> +    qtest_add_func("fuzz/lsi53c895a/lsi_dma_reentrancy",
> > >> +                   test_lsi_dma_reentrancy);
> > >> +
> > >>      return g_test_run();
> > >> }
> > >> --
> > >> 2.39.0
> > >>
> > >>
> > >
> >
> 
> 
> --
> Mauro Matteo Cascella
> Red Hat Product Security
> PGP-Key ID: BB3410B0
> 

Re: [PATCH] scsi/lsi53c895a: restrict DMA engine to memory regions (CVE-2023-0330)

[Thomas Huth]

On 16/01/2023 21.42, Mauro Matteo Cascella wrote:
> This prevents the well known DMA-MMIO reentrancy problem (upstream issue #556)
> leading to memory corruption bugs like stack overflow or use-after-free.
> 
> Fixes: CVE-2023-0330
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> ---

Since the generic reentrancy guard apparently cannot be used for the lsi 
controller (see commit bfd6e7ae6a72b8), I had a try with this patch, ... but 
it seems this breaks the LSI driver of Linux.

I ran QEMU like this:

./qemu-system-x86_64 -accel kvm -m 2G -machine q35 \
  -device lsi53c810,id=lsi1 -device scsi-hd,drive=d0 \
  -drive if=none,id=d0,file=.../somedisk.qcow2 \
  -cdrom Fedora-Everything-netinst-i386-25-1.3.iso

then booted into the rescue shell of the ISO image, and I was not able to 
mount a partition from somedisk.qcow2 anymore. And there were lots of error 
messages related to 53c8... in the "dmesg" output.

It seems like we indeed need some levels of reentrancy here and cannot 
simply disable it completely.

But maybe we can block it at another level. I'll try to come up with a patch...

  Thomas