LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry
@ 2021-07-08  7:46 Harshvardhan Jha
  2021-07-27  3:20 ` Martin K. Petersen
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Harshvardhan Jha @ 2021-07-08  7:46 UTC (permalink / raw)
  To: kashyap.desai
  Cc: sumit.saxena, shivasharan.srikanteshwara, jejb, martin.petersen,
	megaraidlinux.pdl, linux-scsi, linux-kernel, dan.carpenter,
	Harshvardhan Jha

The list_for_each_entry() iterator, "adapter" in this code, can never be
NULL.  If we exit the loop without finding the correct  adapter then
"adapter" points invalid memory that is an offset from the list head.
This will eventually lead to memory corruption and presumably a kernel
crash.

Signed-off-by: Harshvardhan Jha <harshvardhan.jha@oracle.com>
---
From static analysis.  Not tested.
---
 drivers/scsi/megaraid/megaraid_mm.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c
index abf7b401f5b9..c509440bd161 100644
--- a/drivers/scsi/megaraid/megaraid_mm.c
+++ b/drivers/scsi/megaraid/megaraid_mm.c
@@ -238,7 +238,7 @@ mraid_mm_get_adapter(mimd_t __user *umimd, int *rval)
 	mimd_t		mimd;
 	uint32_t	adapno;
 	int		iterator;
-
+	bool		is_found;
 
 	if (copy_from_user(&mimd, umimd, sizeof(mimd_t))) {
 		*rval = -EFAULT;
@@ -254,12 +254,16 @@ mraid_mm_get_adapter(mimd_t __user *umimd, int *rval)
 
 	adapter = NULL;
 	iterator = 0;
+	is_found = false;
 
 	list_for_each_entry(adapter, &adapters_list_g, list) {
-		if (iterator++ == adapno) break;
+		if (iterator++ == adapno) {
+			is_found = true;
+			break;
+		}
 	}
 
-	if (!adapter) {
+	if (!is_found) {
 		*rval = -ENODEV;
 		return NULL;
 	}
@@ -725,6 +729,7 @@ ioctl_done(uioc_t *kioc)
 	uint32_t	adapno;
 	int		iterator;
 	mraid_mmadp_t*	adapter;
+	bool		is_found;
 
 	/*
 	 * When the kioc returns from driver, make sure it still doesn't
@@ -747,19 +752,23 @@ ioctl_done(uioc_t *kioc)
 		iterator	= 0;
 		adapter		= NULL;
 		adapno		= kioc->adapno;
+		is_found	= false;
 
 		con_log(CL_ANN, ( KERN_WARNING "megaraid cmm: completed "
 					"ioctl that was timedout before\n"));
 
 		list_for_each_entry(adapter, &adapters_list_g, list) {
-			if (iterator++ == adapno) break;
+			if (iterator++ == adapno) {
+				is_found = true;
+				break;
+			}
 		}
 
 		kioc->timedout = 0;
 
-		if (adapter) {
+		if (is_found)
 			mraid_mm_dealloc_kioc( adapter, kioc );
-		}
+
 	}
 	else {
 		wake_up(&wait_q);
-- 
2.32.0


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

* Re: [PATCH] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry
  2021-07-08  7:46 [PATCH] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry Harshvardhan Jha
@ 2021-07-27  3:20 ` Martin K. Petersen
  2021-07-27 10:55 ` Sumit Saxena
  2021-07-29  3:37 ` Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2021-07-27  3:20 UTC (permalink / raw)
  To: kashyap.desai
  Cc: Harshvardhan Jha, sumit.saxena, shivasharan.srikanteshwara, jejb,
	martin.petersen, megaraidlinux.pdl, linux-scsi, linux-kernel,
	dan.carpenter


Kashyap,

> The list_for_each_entry() iterator, "adapter" in this code, can never be
> NULL.  If we exit the loop without finding the correct  adapter then
> "adapter" points invalid memory that is an offset from the list head.
> This will eventually lead to memory corruption and presumably a kernel
> crash.

Please review. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

* Re: [PATCH] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry
  2021-07-08  7:46 [PATCH] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry Harshvardhan Jha
  2021-07-27  3:20 ` Martin K. Petersen
@ 2021-07-27 10:55 ` Sumit Saxena
  2021-07-29  3:37 ` Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Sumit Saxena @ 2021-07-27 10:55 UTC (permalink / raw)
  To: Harshvardhan Jha
  Cc: Kashyap Desai, Shivasharan Srikanteshwara, James E.J. Bottomley,
	Martin K. Petersen, PDL,MEGARAIDLINUX, Linux SCSI List, LKML,
	Dan Carpenter

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

On Thu, Jul 8, 2021 at 1:16 PM Harshvardhan Jha
<harshvardhan.jha@oracle.com> wrote:
>
> The list_for_each_entry() iterator, "adapter" in this code, can never be
> NULL.  If we exit the loop without finding the correct  adapter then
> "adapter" points invalid memory that is an offset from the list head.
> This will eventually lead to memory corruption and presumably a kernel
> crash.
>
> Signed-off-by: Harshvardhan Jha <harshvardhan.jha@oracle.com>
Looks good.
Acked-by: Sumit Saxena <sumit.saxena@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry
  2021-07-08  7:46 [PATCH] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry Harshvardhan Jha
  2021-07-27  3:20 ` Martin K. Petersen
  2021-07-27 10:55 ` Sumit Saxena
@ 2021-07-29  3:37 ` Martin K. Petersen
  2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2021-07-29  3:37 UTC (permalink / raw)
  To: Harshvardhan Jha, kashyap.desai
  Cc: Martin K . Petersen, linux-kernel, jejb, linux-scsi,
	dan.carpenter, megaraidlinux.pdl, shivasharan.srikanteshwara,
	sumit.saxena

On Thu, 8 Jul 2021 13:16:42 +0530, Harshvardhan Jha wrote:

> The list_for_each_entry() iterator, "adapter" in this code, can never be
> NULL.  If we exit the loop without finding the correct  adapter then
> "adapter" points invalid memory that is an offset from the list head.
> This will eventually lead to memory corruption and presumably a kernel
> crash.

Applied to 5.14/scsi-fixes, thanks!

[1/1] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry
      https://git.kernel.org/mkp/scsi/c/77541f78eadf

-- 
Martin K. Petersen	Oracle Linux Engineering

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

end of thread, other threads:[~2021-07-29  3:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-08  7:46 [PATCH] scsi: megaraid_mm: Fix end of loop tests for list_for_each_entry Harshvardhan Jha
2021-07-27  3:20 ` Martin K. Petersen
2021-07-27 10:55 ` Sumit Saxena
2021-07-29  3:37 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).