| Message ID | 20200428131939.GA696531@mwanda (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 4a4c0cfb4be74e216dd4446b254594707455bfc6 |
| Headers | show |
| Series | scsi: qedi: Check for buffer overflow in qedi_set_path() | expand |
> -----Original Message----- > From: Dan Carpenter <dan.carpenter@oracle.com> > Sent: Tuesday, April 28, 2020 6:50 PM > To: QLogic-Storage-Upstream@cavium.com; Manish Rangankar > <manish.rangankar@cavium.com> > Cc: James E.J. Bottomley <jejb@linux.ibm.com>; Martin K. Petersen > <martin.petersen@oracle.com>; linux-scsi@vger.kernel.org; kernel- > janitors@vger.kernel.org > Subject: [EXT] [PATCH] scsi: qedi: Check for buffer overflow in > qedi_set_path() > > External Email > > ---------------------------------------------------------------------- > Smatch complains that the "path_data->handle" variable is user controlled. > It comes from iscsi_set_path() so that seems possible. > It's harmless to add a limit check. > > The qedi->ep_tbl[] array has qedi->max_active_conns elements (which is > always ISCSI_MAX_SESS_PER_HBA (4096) elements). The array is allocated > in the qedi_cm_alloc_mem() function. > > Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driver > framework.") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/scsi/qedi/qedi_iscsi.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c > index b867a143d2638..425e665ec08b2 100644 > --- a/drivers/scsi/qedi/qedi_iscsi.c > +++ b/drivers/scsi/qedi/qedi_iscsi.c > @@ -1221,6 +1221,10 @@ static int qedi_set_path(struct Scsi_Host > *shost, struct iscsi_path *path_data) > } > > iscsi_cid = (u32)path_data->handle; > + if (iscsi_cid >= qedi->max_active_conns) { > + ret = -EINVAL; > + goto set_path_exit; > + } > qedi_ep = qedi->ep_tbl[iscsi_cid]; > QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_INFO, > "iscsi_cid=0x%x, qedi_ep=%p\n", iscsi_cid, qedi_ep); Thanks, Acked-by: Manish Rangankar <mrangankar@marvell.com>
On Tue, 28 Apr 2020 16:19:39 +0300, Dan Carpenter wrote: > Smatch complains that the "path_data->handle" variable is user > controlled. It comes from iscsi_set_path() so that seems possible. > It's harmless to add a limit check. > > The qedi->ep_tbl[] array has qedi->max_active_conns elements (which is > always ISCSI_MAX_SESS_PER_HBA (4096) elements). The array is allocated > in the qedi_cm_alloc_mem() function. Applied to 5.8/scsi-queue, thanks! [1/1] scsi: qedi: Check for buffer overflow in qedi_set_path() https://git.kernel.org/mkp/scsi/c/4a4c0cfb4be7
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c index b867a143d2638..425e665ec08b2 100644 --- a/drivers/scsi/qedi/qedi_iscsi.c +++ b/drivers/scsi/qedi/qedi_iscsi.c @@ -1221,6 +1221,10 @@ static int qedi_set_path(struct Scsi_Host *shost, struct iscsi_path *path_data) } iscsi_cid = (u32)path_data->handle; + if (iscsi_cid >= qedi->max_active_conns) { + ret = -EINVAL; + goto set_path_exit; + } qedi_ep = qedi->ep_tbl[iscsi_cid]; QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_INFO, "iscsi_cid=0x%x, qedi_ep=%p\n", iscsi_cid, qedi_ep);
Smatch complains that the "path_data->handle" variable is user controlled. It comes from iscsi_set_path() so that seems possible. It's harmless to add a limit check. The qedi->ep_tbl[] array has qedi->max_active_conns elements (which is always ISCSI_MAX_SESS_PER_HBA (4096) elements). The array is allocated in the qedi_cm_alloc_mem() function. Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driver framework.") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/scsi/qedi/qedi_iscsi.c | 4 ++++ 1 file changed, 4 insertions(+)