linux-hardening - [PATCH v2][next] scsi: mpt3sas: Replace one-element array with flexible-array in struct _MPI2_CONFIG_PAGE_IO_UNIT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 2 Feb 2021 17:51:18 -0600
From:   "Gustavo A. R. Silva" <gustavoars@...nel.org>
To:     Sathya Prakash <sathya.prakash@...adcom.com>,
        Sreekanth Reddy <sreekanth.reddy@...adcom.com>,
        Suganath Prabu Subramani 
        <suganath-prabu.subramani@...adcom.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>
Cc:     MPT-FusionLinux.pdl@...adcom.com, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        linux-hardening@...r.kernel.org
Subject: [PATCH v2][next] scsi: mpt3sas: Replace one-element array with
 flexible-array in struct _MPI2_CONFIG_PAGE_IO_UNIT_3

There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use “flexible array members”[1] for these cases. The older
style of one-element or zero-length arrays should no longer be used[2].

Refactor the code according to the use of a flexible-array member in
struct _MPI2_CONFIG_PAGE_IO_UNIT_3, instead of a one-element array,
and use the struct_size() helper to calculate the size for the
allocation.

Also, this helps the ongoing efforts to enable -Warray-bounds and fix the
following warnings:

drivers/scsi/mpt3sas/mpt3sas_ctl.c:3193:63: warning: array subscript 24
is above array bounds of ‘U16[1]’ {aka ‘short unsigned int[1]’}
[-Warray-bounds]

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays

Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
---
Changes in v2:
 - Fix format specifier: use %zu for size_t type.

 drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 11 +----------
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  6 +++---
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
index 43a3bf8ff428..908b0ca63204 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
@@ -987,21 +987,12 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_1 {
 
 /*IO Unit Page 3 */
 
-/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *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)
-#endif
-
 typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_3 {
 	MPI2_CONFIG_PAGE_HEADER Header;			 /*0x00 */
 	U8                      GPIOCount;		 /*0x04 */
 	U8                      Reserved1;		 /*0x05 */
 	U16                     Reserved2;		 /*0x06 */
-	U16
-		GPIOVal[MPI2_IO_UNIT_PAGE_3_GPIO_VAL_MAX];/*0x08 */
+	U16			GPIOVal[];		 /*0x08 */
 } MPI2_CONFIG_PAGE_IO_UNIT_3,
 	*PTR_MPI2_CONFIG_PAGE_IO_UNIT_3,
 	Mpi2IOUnitPage3_t, *pMpi2IOUnitPage3_t;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index c8a0ce18f2c5..ffb21f873058 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -3143,7 +3143,7 @@ BRM_status_show(struct device *cdev, struct device_attribute *attr,
 	Mpi2ConfigReply_t mpi_reply;
 	u16 backup_rail_monitor_status = 0;
 	u16 ioc_status;
-	int sz;
+	size_t sz;
 	ssize_t rc = 0;
 
 	if (!ioc->is_warpdrive) {
@@ -3157,11 +3157,11 @@ BRM_status_show(struct device *cdev, struct device_attribute *attr,
 		goto out;
 
 	/* allocate upto GPIOVal 36 entries */
-	sz = offsetof(Mpi2IOUnitPage3_t, GPIOVal) + (sizeof(u16) * 36);
+	sz = struct_size(io_unit_pg3, GPIOVal, 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",
+		ioc_err(ioc, "%s: failed allocating memory for iounit_pg3: (%zu) bytes\n",
 			__func__, sz);
 		goto out;
 	}
-- 
2.27.0