| Message ID | 20220705200543.2366809-1-mcascell@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scsi/lsi53c895a: fix use-after-free in lsi_do_msgout (CVE-2022-0216) | expand |
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>
Queued, thanks. Paolo
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
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
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;
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(-)