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

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

Posted by Mauro Matteo Cascella 3 years, 7 months ago

Set current_req->req to NULL to prevent reusing a free'd buffer in case of
repeated SCSI cancel requests. Thanks to Thomas Huth for suggesting the patch.

Fixes: CVE-2022-0216
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
---
 hw/scsi/lsi53c895a.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index c8773f73f7..99ea42d49b 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1028,8 +1028,9 @@ static void lsi_do_msgout(LSIState *s)
         case 0x0d:
             /* The ABORT TAG message clears the current I/O process only. */
             trace_lsi_do_msgout_abort(current_tag);
-            if (current_req) {
+            if (current_req && current_req->req) {
                 scsi_req_cancel(current_req->req);
+                current_req->req = NULL;
             }
             lsi_disconnect(s);
             break;
-- 
2.35.3

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

Posted by Alexander Bulekov 3 years, 7 months ago

On 220705 2205, Mauro Matteo Cascella wrote:
> Set current_req->req to NULL to prevent reusing a free'd buffer in case of
> repeated SCSI cancel requests. Thanks to Thomas Huth for suggesting the patch.
> 
> Fixes: CVE-2022-0216
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> ---
>  hw/scsi/lsi53c895a.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index c8773f73f7..99ea42d49b 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -1028,8 +1028,9 @@ static void lsi_do_msgout(LSIState *s)
>          case 0x0d:
>              /* The ABORT TAG message clears the current I/O process only. */
>              trace_lsi_do_msgout_abort(current_tag);
> -            if (current_req) {
> +            if (current_req && current_req->req) {
>                  scsi_req_cancel(current_req->req);
> +                current_req->req = NULL;
>              }
>              lsi_disconnect(s);
>              break;
> -- 
> 2.35.3
> 
>

Hi Mauro,
https://gitlab.com/qemu-project/qemu/-/issues/972#note_1019851430
This reproducer crashes, with this patch applied. Maybe it is some
different bug though - I'm not sure.

With -trace lsi*

lsi_reg_write Write reg DSP1 0x2d = 0x00
lsi_reg_write Write reg DSP2 0x2e = 0x40
lsi_reg_write Write reg DSP3 0x2f = 0x36
lsi_execute_script SCRIPTS dsp=0x364001d0 opcode 0x58000008 arg 0x0
lsi_execute_script_io_set Set ATN
lsi_execute_script SCRIPTS dsp=0x364001d8 opcode 0x26010000 arg 0x5a41ae0d
lsi_do_msgout MSG out len=65536
lsi_do_msgout_busdevicereset MSG: BUS DEVICE RESET tag=0x0
lsi_do_msgout_select Select LUN 0
lsi_do_msgout_abort MSG: ABORT TAG tag=0x0

In busdevicereset, there are also scsi_req_cancel calls. Do they need
similar changes?

-Alex

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

Posted by Mauro Matteo Cascella 3 years, 7 months ago

Hi Alexander,

Thanks for the reproducer! It looks like ABORT, CLEAR QUEUE and BUS
DEVICE RESET messages can all cancel the current request, so yes I
guess a similar change is needed there, too. Will try to send a v2
soon.

Best regards.


On Sat, Jul 9, 2022 at 2:22 AM Alexander Bulekov <alxndr@bu.edu> wrote:
>
> On 220705 2205, Mauro Matteo Cascella wrote:
> > Set current_req->req to NULL to prevent reusing a free'd buffer in case of
> > repeated SCSI cancel requests. Thanks to Thomas Huth for suggesting the patch.
> >
> > Fixes: CVE-2022-0216
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
> > Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> > ---
> >  hw/scsi/lsi53c895a.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> > index c8773f73f7..99ea42d49b 100644
> > --- a/hw/scsi/lsi53c895a.c
> > +++ b/hw/scsi/lsi53c895a.c
> > @@ -1028,8 +1028,9 @@ static void lsi_do_msgout(LSIState *s)
> >          case 0x0d:
> >              /* The ABORT TAG message clears the current I/O process only. */
> >              trace_lsi_do_msgout_abort(current_tag);
> > -            if (current_req) {
> > +            if (current_req && current_req->req) {
> >                  scsi_req_cancel(current_req->req);
> > +                current_req->req = NULL;
> >              }
> >              lsi_disconnect(s);
> >              break;
> > --
> > 2.35.3
> >
> >
>
> Hi Mauro,
> https://gitlab.com/qemu-project/qemu/-/issues/972#note_1019851430
> This reproducer crashes, with this patch applied. Maybe it is some
> different bug though - I'm not sure.
>
> With -trace lsi*
>
> lsi_reg_write Write reg DSP1 0x2d = 0x00
> lsi_reg_write Write reg DSP2 0x2e = 0x40
> lsi_reg_write Write reg DSP3 0x2f = 0x36
> lsi_execute_script SCRIPTS dsp=0x364001d0 opcode 0x58000008 arg 0x0
> lsi_execute_script_io_set Set ATN
> lsi_execute_script SCRIPTS dsp=0x364001d8 opcode 0x26010000 arg 0x5a41ae0d
> lsi_do_msgout MSG out len=65536
> lsi_do_msgout_busdevicereset MSG: BUS DEVICE RESET tag=0x0
> lsi_do_msgout_select Select LUN 0
> lsi_do_msgout_abort MSG: ABORT TAG tag=0x0
>
> In busdevicereset, there are also scsi_req_cancel calls. Do they need
> similar changes?
>
> -Alex
>


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

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

Posted by Paolo Bonzini 3 years, 7 months ago

Queued, thanks.

Paolo

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

Posted by Thomas Huth 3 years, 7 months ago

On 05/07/2022 22.05, Mauro Matteo Cascella wrote:
> Set current_req->req to NULL to prevent reusing a free'd buffer in case of
> repeated SCSI cancel requests. Thanks to Thomas Huth for suggesting the patch.
> 
> Fixes: CVE-2022-0216
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/972
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> ---
>   hw/scsi/lsi53c895a.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index c8773f73f7..99ea42d49b 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -1028,8 +1028,9 @@ static void lsi_do_msgout(LSIState *s)
>           case 0x0d:
>               /* The ABORT TAG message clears the current I/O process only. */
>               trace_lsi_do_msgout_abort(current_tag);
> -            if (current_req) {
> +            if (current_req && current_req->req) {
>                   scsi_req_cancel(current_req->req);
> +                current_req->req = NULL;
>               }
>               lsi_disconnect(s);
>               break;

Let's hope that this will fix the issue for good...

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