| Message ID | 20211112120641.16073-1-fantasquex@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: qedf: Fix a UAF bug in __qedf_probe | expand |
Hi Letu Ren, > -----Original Message----- > From: Letu Ren <fantasquex@gmail.com> > Sent: Friday, November 12, 2021 5:37 PM > To: Saurav Kashyap <skashyap@marvell.com>; Javed Hasan > <jhasan@marvell.com>; jejb@linux.ibm.com; martin.petersen@oracle.com > Cc: GR-QLogic-Storage-Upstream <GR-QLogic-Storage- > Upstream@marvell.com>; linux-scsi@vger.kernel.org; linux- > kernel@vger.kernel.org; Letu Ren <fantasquex@gmail.com>; Zheyu Ma > <zheyuma97@gmail.com>; Wende Tan <twd2.me@gmail.com> > Subject: [EXT] [PATCH] scsi: qedf: Fix a UAF bug in __qedf_probe > > External Email > > ---------------------------------------------------------------------- > In __qedf_probe, if `qedf->cdev` is NULL which means > qed_ops->common->probe() failed, then the program will goto label err1, > scsi_host_put() will free `lport->host` pointer. Because the memory `qedf` > points to is allocated by libfc_host_alloc(), it will be freed by > scsi_host_put(). However, the if statement below label err0 only checks > whether qedf is NULL but doesn't check whether the memory has been freed. > So a UAF bug occurred. > > There are two ways to get to the statements below err0. the first one is > described as before. `qedf` should be set to NULL. The second way is goto > `err0` directly. In this way `qedf` hasn't been changed and it has the > initial value `NULL`. So the program wont't go to the if statement in any > situation. > > The KASAN logs are as follows: > > [ 2.312969] BUG: KASAN: use-after-free in __qedf_probe+0x5dcf/0x6bc0 > [ 2.312969] > [ 2.312969] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 > [ 2.312969] Call Trace: > [ 2.312969] dump_stack_lvl+0x59/0x7b > [ 2.312969] print_address_description+0x7c/0x3b0 > [ 2.312969] ? __qedf_probe+0x5dcf/0x6bc0 > [ 2.312969] __kasan_report+0x160/0x1c0 > [ 2.312969] ? __qedf_probe+0x5dcf/0x6bc0 > [ 2.312969] kasan_report+0x4b/0x70 > [ 2.312969] ? kobject_put+0x25d/0x290 > [ 2.312969] kasan_check_range+0x2ca/0x310 > [ 2.312969] __qedf_probe+0x5dcf/0x6bc0 > [ 2.312969] ? selinux_kernfs_init_security+0xdc/0x5f0 > [ 2.312969] ? trace_rpm_return_int_rcuidle+0x18/0x120 > [ 2.312969] ? rpm_resume+0xa5c/0x16e0 > [ 2.312969] ? qedf_get_generic_tlv_data+0x160/0x160 > [ 2.312969] local_pci_probe+0x13c/0x1f0 > [ 2.312969] pci_device_probe+0x37e/0x6c0 > > Reported-by: Zheyu Ma <zheyuma97@gmail.com> > Signed-off-by: Letu Ren <fantasquex@gmail.com> > Co-developed-by: Wende Tan <twd2.me@gmail.com> > Signed-off-by: Wende Tan <twd2.me@gmail.com> > --- > drivers/scsi/qedf/qedf_main.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c > index 42d0d941dba5..3ea552acd82a 100644 > --- a/drivers/scsi/qedf/qedf_main.c > +++ b/drivers/scsi/qedf/qedf_main.c > @@ -3683,11 +3683,6 @@ static int __qedf_probe(struct pci_dev *pdev, int > mode) > err1: > scsi_host_put(lport->host); > err0: > - if (qedf) { > - QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_DISC, "Probe > done.\n"); > - > - clear_bit(QEDF_PROBING, &qedf->flags); > - } > return rc; > } Thanks for the patch. Acked-by: Saurav Kashyap <skashyap@marvell.com> Thanks, ~Saurav > > -- > 2.33.1
Hi Saurav, > Thanks for the patch. > > Acked-by: Saurav Kashyap <skashyap@marvell.com> This is a friendly reminder. I wonder the review progress of this patch. Almost a year has passed since I submitted the patch. And we have tested qedf on Linux 6.0-rc0. This UAF problem still exists. If there's anything I can do to help with this issue please let me know. I'm willing to help. Letu Ren
Hi Letu Ren, I have already ack-ed this patch, so nothing to be done from my side. Hi Martin, I have ack-ed this patch, kindly pull it. Thanks, ~saurav > -----Original Message----- > From: Letu Ren <fantasquex@gmail.com> > Sent: Tuesday, September 6, 2022 3:31 PM > To: Saurav Kashyap <skashyap@marvell.com> > Cc: Javed Hasan <jhasan@marvell.com>; jejb@linux.ibm.com; > martin.petersen@oracle.com; GR-QLogic-Storage-Upstream <GR-QLogic- > Storage-Upstream@marvell.com>; linux-scsi@vger.kernel.org; linux- > kernel@vger.kernel.org; Zheyu Ma <zheyuma97@gmail.com>; Wende Tan > <twd2.me@gmail.com> > Subject: Re: [EXT] [PATCH] scsi: qedf: Fix a UAF bug in __qedf_probe > > Hi Saurav, > > > Thanks for the patch. > > > > Acked-by: Saurav Kashyap <skashyap@marvell.com> > > > This is a friendly reminder. I wonder the review progress of this > patch. Almost a year has passed since I submitted the patch. And we > have tested qedf on Linux 6.0-rc0. This UAF problem still exists. If > there's anything I can do to help with this issue please let me know. > I'm willing to help. > > Letu Ren
On Fri, 12 Nov 2021 20:06:41 +0800, Letu Ren wrote: > In __qedf_probe, if `qedf->cdev` is NULL which means > qed_ops->common->probe() failed, then the program will goto label err1, > scsi_host_put() will free `lport->host` pointer. Because the memory `qedf` > points to is allocated by libfc_host_alloc(), it will be freed by > scsi_host_put(). However, the if statement below label err0 only checks > whether qedf is NULL but doesn't check whether the memory has been freed. > So a UAF bug occurred. > > [...] Applied to 6.0/scsi-fixes, thanks! [1/1] scsi: qedf: Fix a UAF bug in __qedf_probe https://git.kernel.org/mkp/scsi/c/fbfe96869b78
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c index 42d0d941dba5..3ea552acd82a 100644 --- a/drivers/scsi/qedf/qedf_main.c +++ b/drivers/scsi/qedf/qedf_main.c @@ -3683,11 +3683,6 @@ static int __qedf_probe(struct pci_dev *pdev, int mode) err1: scsi_host_put(lport->host); err0: - if (qedf) { - QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_DISC, "Probe done.\n"); - - clear_bit(QEDF_PROBING, &qedf->flags); - } return rc; }