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 <[email protected]>
---
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
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
On Thu, Jul 8, 2021 at 1:16 PM Harshvardhan Jha
<[email protected]> 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 <[email protected]>
Looks good.
Acked-by: Sumit Saxena <[email protected]>
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