All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: qedi: Check for buffer overflow in qedi_set_path()
@ 2020-04-28 13:19 ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2020-04-28 13:19 UTC (permalink / raw)
  To: QLogic-Storage-Upstream, Manish Rangankar
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, kernel-janitors

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);
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] scsi: qedi: Check for buffer overflow in qedi_set_path()
@ 2020-04-28 13:19 ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2020-04-28 13:19 UTC (permalink / raw)
  To: QLogic-Storage-Upstream, Manish Rangankar
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, kernel-janitors

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);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [EXT] [PATCH] scsi: qedi: Check for buffer overflow in qedi_set_path()
  2020-04-28 13:19 ` Dan Carpenter
@ 2020-04-29  5:48   ` Manish Rangankar
  -1 siblings, 0 replies; 6+ messages in thread
From: Manish Rangankar @ 2020-04-29  5:48 UTC (permalink / raw)
  To: Dan Carpenter, QLogic-Storage-Upstream, Manish Rangankar
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, kernel-janitors

> -----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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [EXT] [PATCH] scsi: qedi: Check for buffer overflow in qedi_set_path()
@ 2020-04-29  5:48   ` Manish Rangankar
  0 siblings, 0 replies; 6+ messages in thread
From: Manish Rangankar @ 2020-04-29  5:48 UTC (permalink / raw)
  To: Dan Carpenter, QLogic-Storage-Upstream, Manish Rangankar
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, kernel-janitors

> -----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>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: qedi: Check for buffer overflow in qedi_set_path()
  2020-04-28 13:19 ` Dan Carpenter
@ 2020-04-30  2:18   ` Martin K. Petersen
  -1 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2020-04-30  2:18 UTC (permalink / raw)
  To: QLogic-Storage-Upstream, Dan Carpenter, Manish Rangankar
  Cc: Martin K . Petersen, linux-scsi, James E.J. Bottomley, kernel-janitors

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

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: qedi: Check for buffer overflow in qedi_set_path()
@ 2020-04-30  2:18   ` Martin K. Petersen
  0 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2020-04-30  2:18 UTC (permalink / raw)
  To: QLogic-Storage-Upstream, Dan Carpenter, Manish Rangankar
  Cc: Martin K . Petersen, linux-scsi, James E.J. Bottomley, kernel-janitors

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

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-04-30  2:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-28 13:19 [PATCH] scsi: qedi: Check for buffer overflow in qedi_set_path() Dan Carpenter
2020-04-28 13:19 ` Dan Carpenter
2020-04-29  5:48 ` [EXT] " Manish Rangankar
2020-04-29  5:48   ` Manish Rangankar
2020-04-30  2:18 ` Martin K. Petersen
2020-04-30  2:18   ` Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.