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] 
> 20230313082417.827484-1-alxndr@bu.edu/20230313082417.827484-7-alxndr@bu.edu/">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
>