Re: [PATCH] scsi/lsi53c895a: really fix use-after-free in lsi_do_msgout (CVE-2022-0216)

[Bin Meng]

Hi,

On Wed, Jul 13, 2022 at 8:45 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Mauro Matteo Cascella <mcascell@redhat.com>
>
> Set current_req to NULL, not current_req->req, to prevent reusing a free'd
> buffer in case of repeated SCSI cancel requests.  Also apply the fix to
> CLEAR QUEUE and BUS DEVICE RESET messages as well, since they also cancel
> the request.
>
> Thanks to Alexander Bulekov for providing a reproducer.
>
> Fixes: CVE-2022-0216
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Tested-by: Alexander Bulekov <alxndr@bu.edu>
> Message-Id: <20220711123316.421279-1-mcascell@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         Adjust the patch from v1 to v2 since the changes crossed
>         with the pull request.
>
>  hw/scsi/lsi53c895a.c               |  3 +-
>  tests/qtest/fuzz-lsi53c895a-test.c | 71 ++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 99ea42d49b..ad5f5e5f39 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -1030,7 +1030,7 @@ static void lsi_do_msgout(LSIState *s)
>              trace_lsi_do_msgout_abort(current_tag);
>              if (current_req && current_req->req) {
>                  scsi_req_cancel(current_req->req);
> -                current_req->req = NULL;
> +                current_req = NULL;
>              }
>              lsi_disconnect(s);
>              break;
> @@ -1056,6 +1056,7 @@ static void lsi_do_msgout(LSIState *s)
>              /* clear the current I/O process */
>              if (s->current) {
>                  scsi_req_cancel(s->current->req);
> +                current_req = NULL;
>              }
>
>              /* As the current implemented devices scsi_disk and scsi_generic
> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c 
> b/tests/qtest/fuzz-lsi53c895a-test.c
> index 2e8e67859e..6872c70d3a 100644
> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> @@ -8,6 +8,74 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>
> +/*
> + * This used to trigger a UAF in lsi_do_msgout()
> + * https://gitlab.com/qemu-project/qemu/-/issues/972
> + */
> +static void test_lsi_do_msgout_cancel_req(void)
> +{
> +    QTestState *s;
> +
> +    s = qtest_init("-M q35 -m 4G -display none -nodefaults "

This test does not run on machines with small size memory. Is 4G a
must have for this test?

> +                   "-device lsi53c895a,id=scsi "
> +                   "-device scsi-hd,drive=disk0 "
> +                   "-drive file=null-co://,id=disk0,if=none,format=raw");
> +
> +    qtest_outl(s, 0xcf8, 0x80000810);
> +    qtest_outl(s, 0xcf8, 0xc000);
> +    qtest_outl(s, 0xcf8, 0x80000810);
> +    qtest_outw(s, 0xcfc, 0x7);
> +    qtest_outl(s, 0xcf8, 0x80000810);
> +    qtest_outl(s, 0xcfc, 0xc000);
> +    qtest_outl(s, 0xcf8, 0x80000804);
> +    qtest_outw(s, 0xcfc, 0x05);
> +    qtest_writeb(s, 0x69736c10, 0x08);
> +    qtest_writeb(s, 0x69736c13, 0x58);
> +    qtest_writeb(s, 0x69736c1a, 0x01);
> +    qtest_writeb(s, 0x69736c1b, 0x06);
> +    qtest_writeb(s, 0x69736c22, 0x01);
> +    qtest_writeb(s, 0x69736c23, 0x07);
> +    qtest_writeb(s, 0x69736c2b, 0x02);
> +    qtest_writeb(s, 0x69736c48, 0x08);
> +    qtest_writeb(s, 0x69736c4b, 0x58);
> +    qtest_writeb(s, 0x69736c52, 0x04);
> +    qtest_writeb(s, 0x69736c53, 0x06);
> +    qtest_writeb(s, 0x69736c5b, 0x02);
> +    qtest_outl(s, 0xc02d, 0x697300);
> +    qtest_writeb(s, 0x5a554662, 0x01);
> +    qtest_writeb(s, 0x5a554663, 0x07);
> +    qtest_writeb(s, 0x5a55466a, 0x10);
> +    qtest_writeb(s, 0x5a55466b, 0x22);
> +    qtest_writeb(s, 0x5a55466c, 0x5a);
> +    qtest_writeb(s, 0x5a55466d, 0x5a);
> +    qtest_writeb(s, 0x5a55466e, 0x34);
> +    qtest_writeb(s, 0x5a55466f, 0x5a);
> +    qtest_writeb(s, 0x5a345a5a, 0x77);
> +    qtest_writeb(s, 0x5a345a5b, 0x55);
> +    qtest_writeb(s, 0x5a345a5c, 0x51);
> +    qtest_writeb(s, 0x5a345a5d, 0x27);
> +    qtest_writeb(s, 0x27515577, 0x41);
> +    qtest_outl(s, 0xc02d, 0x5a5500);
> +    qtest_writeb(s, 0x364001d0, 0x08);
> +    qtest_writeb(s, 0x364001d3, 0x58);
> +    qtest_writeb(s, 0x364001da, 0x01);
> +    qtest_writeb(s, 0x364001db, 0x26);
> +    qtest_writeb(s, 0x364001dc, 0x0d);
> +    qtest_writeb(s, 0x364001dd, 0xae);
> +    qtest_writeb(s, 0x364001de, 0x41);
> +    qtest_writeb(s, 0x364001df, 0x5a);
> +    qtest_writeb(s, 0x5a41ae0d, 0xf8);
> +    qtest_writeb(s, 0x5a41ae0e, 0x36);
> +    qtest_writeb(s, 0x5a41ae0f, 0xd7);
> +    qtest_writeb(s, 0x5a41ae10, 0x36);
> +    qtest_writeb(s, 0x36d736f8, 0x0c);
> +    qtest_writeb(s, 0x36d736f9, 0x80);
> +    qtest_writeb(s, 0x36d736fa, 0x0d);
> +    qtest_outl(s, 0xc02d, 0x364000);
> +
> +    qtest_quit(s);
> +}
> +
>  /*
>   * This used to trigger the assert in lsi_do_dma()
>   * https://bugs.launchpad.net/qemu/+bug/697510
> @@ -44,5 +112,8 @@ int main(int argc, char **argv)
>      qtest_add_func("fuzz/lsi53c895a/lsi_do_dma_empty_queue",
>                     test_lsi_do_dma_empty_queue);
>
> +    qtest_add_func("fuzz/lsi53c895a/lsi_do_msgout_cancel_req",
> +                   test_lsi_do_msgout_cancel_req);
> +
>      return g_test_run();
>  }

Regards,
Bin