From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Sathya Prakash <sathya.prakash@broadcom.com>,
Sreekanth Reddy <sreekanth.reddy@broadcom.com>,
Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>,
"James E.J. Bottomley" <jejb@linux.ibm.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: MPT-FusionLinux.pdl@broadcom.com, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
linux-hardening@vger.kernel.org
Subject: [PATCH][next] scsi: mpt3sas: Replace unnecessary dynamic allocation with a static one
Date: Wed, 10 Mar 2021 17:59:51 -0600 [thread overview]
Message-ID: <20210310235951.GA108661@embeddedor> (raw)
Dynamic memory allocation isn't actually needed and it can be
replaced by statically allocating memory for struct object
io_unit_pg3 with 36 hardcoded entries for its GPIOVal array.
Also, this helps the with ongoing efforts to enable -Warray-bounds
by fixing the following warning:
drivers/scsi/mpt3sas/mpt3sas_ctl.c: In function ‘BRM_status_show’:
drivers/scsi/mpt3sas/mpt3sas_ctl.c:3257:63: warning: array subscript 24 is above array bounds of ‘U16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds]
3257 | backup_rail_monitor_status = le16_to_cpu(io_unit_pg3->GPIOVal[24]);
./include/uapi/linux/byteorder/little_endian.h:36:51: note: in definition of macro ‘__le16_to_cpu’
36 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
drivers/scsi/mpt3sas/mpt3sas_ctl.c:3257:31: note: in expansion of macro ‘le16_to_cpu’
3257 | backup_rail_monitor_status = le16_to_cpu(io_unit_pg3->GPIOVal[24]);
Link: https://github.com/KSPP/linux/issues/109
Link: https://lore.kernel.org/lkml/202103101058.16ED27BE3@keescook/
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 2 +-
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 24 ++++++++----------------
2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
index 43a3bf8ff428..d00431f553e1 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
@@ -992,7 +992,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_1 {
*one and check the value returned for GPIOCount at runtime.
*/
#ifndef MPI2_IO_UNIT_PAGE_3_GPIO_VAL_MAX
-#define MPI2_IO_UNIT_PAGE_3_GPIO_VAL_MAX (1)
+#define MPI2_IO_UNIT_PAGE_3_GPIO_VAL_MAX (36)
#endif
typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_3 {
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 44f9a05db94e..bc56e058a523 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -3203,7 +3203,7 @@ BRM_status_show(struct device *cdev, struct device_attribute *attr,
{
struct Scsi_Host *shost = class_to_shost(cdev);
struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
- Mpi2IOUnitPage3_t *io_unit_pg3 = NULL;
+ Mpi2IOUnitPage3_t io_unit_pg3;
Mpi2ConfigReply_t mpi_reply;
u16 backup_rail_monitor_status = 0;
u16 ioc_status;
@@ -3220,17 +3220,10 @@ BRM_status_show(struct device *cdev, struct device_attribute *attr,
if (ioc->pci_error_recovery || ioc->remove_host)
goto out;
- /* allocate upto GPIOVal 36 entries */
- sz = offsetof(Mpi2IOUnitPage3_t, GPIOVal) + (sizeof(u16) * 36);
- io_unit_pg3 = kzalloc(sz, GFP_KERNEL);
- if (!io_unit_pg3) {
- rc = -ENOMEM;
- ioc_err(ioc, "%s: failed allocating memory for iounit_pg3: (%d) bytes\n",
- __func__, sz);
- goto out;
- }
+ sz = sizeof(io_unit_pg3);
+ memset(&io_unit_pg3, 0, sz);
- if (mpt3sas_config_get_iounit_pg3(ioc, &mpi_reply, io_unit_pg3, sz) !=
+ if (mpt3sas_config_get_iounit_pg3(ioc, &mpi_reply, &io_unit_pg3, sz) !=
0) {
ioc_err(ioc, "%s: failed reading iounit_pg3\n",
__func__);
@@ -3246,19 +3239,18 @@ BRM_status_show(struct device *cdev, struct device_attribute *attr,
goto out;
}
- if (io_unit_pg3->GPIOCount < 25) {
- ioc_err(ioc, "%s: iounit_pg3->GPIOCount less than 25 entries, detected (%d) entries\n",
- __func__, io_unit_pg3->GPIOCount);
+ if (io_unit_pg3.GPIOCount < 25) {
+ ioc_err(ioc, "%s: iounit_pg3.GPIOCount less than 25 entries, detected (%d) entries\n",
+ __func__, io_unit_pg3.GPIOCount);
rc = -EINVAL;
goto out;
}
/* BRM status is in bit zero of GPIOVal[24] */
- backup_rail_monitor_status = le16_to_cpu(io_unit_pg3->GPIOVal[24]);
+ backup_rail_monitor_status = le16_to_cpu(io_unit_pg3.GPIOVal[24]);
rc = snprintf(buf, PAGE_SIZE, "%d\n", (backup_rail_monitor_status & 1));
out:
- kfree(io_unit_pg3);
mutex_unlock(&ioc->pci_access_mutex);
return rc;
}
--
2.27.0
next reply other threads:[~2021-03-11 0:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-10 23:59 Gustavo A. R. Silva [this message]
2021-03-17 3:32 ` [PATCH][next] scsi: mpt3sas: Replace unnecessary dynamic allocation with a static one Martin K. Petersen
2021-03-17 2:41 ` Gustavo A. R. Silva
2021-03-19 3:46 ` Martin K. Petersen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210310235951.GA108661@embeddedor \
--to=gustavoars@kernel.org \
--cc=MPT-FusionLinux.pdl@broadcom.com \
--cc=jejb@linux.ibm.com \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=sathya.prakash@broadcom.com \
--cc=sreekanth.reddy@broadcom.com \
--cc=suganath-prabu.subramani@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.