| Message ID | 20201224175441.67538-1-mcascell@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/scsi/megasas: check for NULL frame in megasas_command_cancelled() | expand |
Looks like one reported by OSS-Fuzz: Here's a reproducer cat << EOF | ./qemu-system-i386 -qtest stdio -display none \ -machine q35,accel=qtest -m 512M -nodefaults \ -device megasas -device scsi-cd,drive=null0 \ -blockdev driver=null-co,read-zeroes=on,node-name=null0 outl 0xcf8 0x80000801 outl 0xcfc 0x15000000 outl 0xcf8 0x80000817 outl 0xcfc 0x1e0000 write 0x40 0x1 0x01 write 0x47 0x1 0x03 write 0x50 0x1 0x12 write 0x55 0x1 0x10 write 0x6a 0x1 0x20 write 0x70 0x1 0x10 write 0x7b 0x1 0x10 write 0x7f 0x1 0x10 write 0x86 0x1 0x10 write 0x8b 0x1 0x10 outb 0x1e40 0x40 write 0x1a 0x1 0x0 write 0x6a000f 0x1 0x0 outb 0x1e40 0x0 outl 0x1e40 0x0 write 0x6f1 0x1 0x00 write 0x6f9 0x1 0x00 write 0x6fd 0x1 0x01 write 0x701 0x1 0x00 write 0x705 0x1 0x06 write 0x730 0x1 0x00 write 0x738 0x1 0x00 write 0x73c 0x1 0x01 write 0x740 0x1 0x00 write 0x744 0x1 0x06 write 0x75c 0x1 0x00 write 0x760 0x1 0x01 write 0x76f 0x1 0x00 write 0x770 0x1 0x20 write 0x77c 0x1 0x20 write 0x780 0x1 0x00 write 0x79b 0x1 0x00 write 0x79f 0x1 0x01 write 0x7ae 0x1 0x00 write 0x7af 0x1 0x20 write 0x7bb 0x1 0x20 write 0x7bf 0x1 0x00 write 0x7cf 0x1 0x10 write 0x7db 0x1 0x00 write 0x7df 0x1 0x20 write 0x7ee 0x1 0x20 write 0x7ef 0x1 0x06 write 0x7fb 0x1 0x10 write 0x7ff 0x1 0x00 outb 0x1e40 0x0 outl 0x1e1f 0x40000200 EOF -Alex On 201224 1854, Mauro Matteo Cascella wrote: > Ensure that 'cmd->frame' is not NULL before accessing the 'header' field. > This check prevents a potential NULL pointer dereference issue. > > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1910346 > Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com> > Reported-by: Cheolwoo Myung <cwmyung@snu.ac.kr> > --- > hw/scsi/megasas.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c > index 1a5fc5857d..77510e120c 100644 > --- a/hw/scsi/megasas.c > +++ b/hw/scsi/megasas.c > @@ -1893,7 +1893,7 @@ static void megasas_command_cancelled(SCSIRequest *req) > { > MegasasCmd *cmd = req->hba_private; > > - if (!cmd) { > + if (!cmd || !cmd->frame) { > return; > } > cmd->frame->header.cmd_status = MFI_STAT_SCSI_IO_FAILED; > -- > 2.29.2 > >
Hello, Any updates on this little patch? Please find below a reproducer for this bug (thanks Alexander): https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg02567.html Thank you, On Thu, Dec 24, 2020 at 6:55 PM Mauro Matteo Cascella <mcascell@redhat.com> wrote: > > Ensure that 'cmd->frame' is not NULL before accessing the 'header' field. > This check prevents a potential NULL pointer dereference issue. > > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1910346 > Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com> > Reported-by: Cheolwoo Myung <cwmyung@snu.ac.kr> > --- > hw/scsi/megasas.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c > index 1a5fc5857d..77510e120c 100644 > --- a/hw/scsi/megasas.c > +++ b/hw/scsi/megasas.c > @@ -1893,7 +1893,7 @@ static void megasas_command_cancelled(SCSIRequest *req) > { > MegasasCmd *cmd = req->hba_private; > > - if (!cmd) { > + if (!cmd || !cmd->frame) { > return; > } > cmd->frame->header.cmd_status = MFI_STAT_SCSI_IO_FAILED; > -- > 2.29.2 >
You forgot to Cc the subsystem maintainers... ./scripts/get_maintainer.pl -f hw/scsi/megasas.c Hannes Reinecke <hare@suse.com> (supporter:megasas) Paolo Bonzini <pbonzini@redhat.com> (supporter:SCSI) Fam Zheng <fam@euphon.net> (reviewer:SCSI) On 1/25/21 3:22 PM, Mauro Matteo Cascella wrote: > Hello, > > Any updates on this little patch? Please find below a reproducer for > this bug (thanks Alexander): > https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg02567.html "Little patch" but with security involvements ;) As Paolo usually asks for reproducer to be integrated with the fix, it might save him/you time if you respin with the reproducer. You can have a look at https://www.mail-archive.com/qemu-block@nongnu.org/msg78982.html for example. That said, unrelated to your patch but I'm not sure how useful it is to test for bugs found by fuzzer each time in our CI. There are borderline cases not representing proper use. Maybe we could run them weekly instead... > Thank you, > > On Thu, Dec 24, 2020 at 6:55 PM Mauro Matteo Cascella > <mcascell@redhat.com> wrote: >> >> Ensure that 'cmd->frame' is not NULL before accessing the 'header' field. >> This check prevents a potential NULL pointer dereference issue. >> >> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1910346 >> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com> >> Reported-by: Cheolwoo Myung <cwmyung@snu.ac.kr> >> --- >> hw/scsi/megasas.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c >> index 1a5fc5857d..77510e120c 100644 >> --- a/hw/scsi/megasas.c >> +++ b/hw/scsi/megasas.c >> @@ -1893,7 +1893,7 @@ static void megasas_command_cancelled(SCSIRequest *req) >> { >> MegasasCmd *cmd = req->hba_private; >> >> - if (!cmd) { >> + if (!cmd || !cmd->frame) { >> return; >> } >> cmd->frame->header.cmd_status = MFI_STAT_SCSI_IO_FAILED; >> -- >> 2.29.2 >> > >
On Mon, Jan 25, 2021 at 3:52 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > You forgot to Cc the subsystem maintainers... > > ./scripts/get_maintainer.pl -f hw/scsi/megasas.c > Hannes Reinecke <hare@suse.com> (supporter:megasas) > Paolo Bonzini <pbonzini@redhat.com> (supporter:SCSI) > Fam Zheng <fam@euphon.net> (reviewer:SCSI) I used to only check the MAINTAINERS file, which only mentions Hannes in connection with megasas. Good to know for the nex time. > As Paolo usually asks for reproducer to be integrated with the fix, > it might save him/you time if you respin with the reproducer. You > can have a look at > https://www.mail-archive.com/qemu-block@nongnu.org/msg78982.html > for example. Thank you for the heads up, Philippe. I'll try to incorporate the reproducer. > That said, unrelated to your patch but I'm not sure how useful it > is to test for bugs found by fuzzer each time in our CI. There are > borderline cases not representing proper use. Maybe we could run > them weekly instead... -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index 1a5fc5857d..77510e120c 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -1893,7 +1893,7 @@ static void megasas_command_cancelled(SCSIRequest *req) { MegasasCmd *cmd = req->hba_private; - if (!cmd) { + if (!cmd || !cmd->frame) { return; } cmd->frame->header.cmd_status = MFI_STAT_SCSI_IO_FAILED;
Ensure that 'cmd->frame' is not NULL before accessing the 'header' field. This check prevents a potential NULL pointer dereference issue. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1910346 Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com> Reported-by: Cheolwoo Myung <cwmyung@snu.ac.kr> --- hw/scsi/megasas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)