* [PATCH 0/6] scsi: mpt3sas: Use flexible arrays and do a few cleanups
@ 2023-07-25 16:13 James Seo
2023-07-25 16:13 ` [PATCH 1/6] scsi: mpt3sas: Use flexible arrays when obviously possible James Seo
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: James Seo @ 2023-07-25 16:13 UTC (permalink / raw)
To: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani
Cc: James Seo, James E.J. Bottomley, Martin K. Petersen, Kees Cook,
Gustavo A. R. Silva, MPT-FusionLinux.pdl, linux-scsi,
linux-kernel
Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") has
resulted in the only arrays that UBSAN_BOUNDS considers unbounded
being trailing arrays declared with [] as the last member of a
struct.
Unbounded trailing arrays declared with [1] are common in mpt3sas,
which is causing spurious warnings to appear in some situations with
UBSAN array bounds checks enabled, e.g. when more than one physical
disk is connected:
UBSAN: array-index-out-of-bounds in drivers/scsi/mpt3sas/mpt3sas_scsih.c:6810:36
index 1 is out of range for type 'MPI2_SAS_IO_UNIT0_PHY_DATA [1]'
which relates to this unbounded array access:
port_id = sas_iounit_pg0->PhyData[i].Port;
and is just one example of 10 currently occurring for me during boot.
This series converts most trailing arrays in mpt3sas into proper C99
flexible array members. Those that really are fixed-length arrays of
length >=1 are left alone.
I didn't find any conversions that required further source edits
besides changing [1] to [], and everything seems to work with my
SAS2008-based add-in card, but please look things over in case I
missed something subtle.
Rounding out the series are some opportunistic cleanups.
James Seo (6):
scsi: mpt3sas: Use flexible arrays when obviously possible
scsi: mpt3sas: Use flexible arrays when less obviously possible
scsi: mpt3sas: Use struct_size() for struct size calculations
scsi: mpt3sas: Fix an outdated comment
scsi: mpt3sas: Fix typo of "TRIGGER"
scsi: mpt3sas: Replace a dynamic allocation with a local variable
drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 231 ++++++-------------
drivers/scsi/mpt3sas/mpi/mpi2_image.h | 32 +--
drivers/scsi/mpt3sas/mpi/mpi2_ioc.h | 27 +--
drivers/scsi/mpt3sas/mpt3sas_base.c | 28 +--
drivers/scsi/mpt3sas/mpt3sas_config.c | 6 +-
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 18 +-
drivers/scsi/mpt3sas/mpt3sas_transport.c | 9 +-
drivers/scsi/mpt3sas/mpt3sas_trigger_pages.h | 44 ++--
drivers/scsi/mpt3sas/mpt3sas_warpdrive.c | 3 +-
9 files changed, 135 insertions(+), 263 deletions(-)
base-commit: 7e9609d2daea0ebe4add444b26b76479ecfda504
--
2.39.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/6] scsi: mpt3sas: Use flexible arrays when obviously possible
2023-07-25 16:13 [PATCH 0/6] scsi: mpt3sas: Use flexible arrays and do a few cleanups James Seo
@ 2023-07-25 16:13 ` James Seo
2023-07-28 22:01 ` Kees Cook
2023-07-25 16:13 ` [PATCH 2/6] scsi: mpt3sas: Use flexible arrays when less " James Seo
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: James Seo @ 2023-07-25 16:13 UTC (permalink / raw)
To: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani
Cc: James Seo, James E.J. Bottomley, Martin K. Petersen, Kees Cook,
Gustavo A. R. Silva, MPT-FusionLinux.pdl, linux-scsi,
linux-kernel
These old-style 1-length variable arrays can be directly converted
into C99 flexible array members without any binary changes.
In most cases, they belong to unused structs, or to structs used only
by unused code. The remaining few coincidentally have their sizes
calculated in roundabout ways that do not depend on the sizeof()
their structs.
Signed-off-by: James Seo <james@equiv.tech>
---
drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 181 ++++++++------------------
drivers/scsi/mpt3sas/mpi/mpi2_image.h | 32 ++---
drivers/scsi/mpt3sas/mpi/mpi2_ioc.h | 27 ++--
3 files changed, 75 insertions(+), 165 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
index 4d0be5ab98c1..42d820159c44 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
@@ -643,18 +643,14 @@ typedef struct _MPI2_CHIP_REVISION_ID {
/*Manufacturing Page 2 */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check Header.PageLength at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check Header.PageLength at
+ *runtime before using HwSettings[].
*/
-#ifndef MPI2_MAN_PAGE_2_HW_SETTINGS_WORDS
-#define MPI2_MAN_PAGE_2_HW_SETTINGS_WORDS (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_MAN_2 {
MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
MPI2_CHIP_REVISION_ID ChipId; /*0x04 */
- U32
- HwSettings[MPI2_MAN_PAGE_2_HW_SETTINGS_WORDS];/*0x08 */
+ U32 HwSettings[]; /*0x08 */
} MPI2_CONFIG_PAGE_MAN_2,
*PTR_MPI2_CONFIG_PAGE_MAN_2,
Mpi2ManufacturingPage2_t,
@@ -666,18 +662,14 @@ typedef struct _MPI2_CONFIG_PAGE_MAN_2 {
/*Manufacturing Page 3 */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check Header.PageLength at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check Header.PageLength at
+ *runtime before using Info[].
*/
-#ifndef MPI2_MAN_PAGE_3_INFO_WORDS
-#define MPI2_MAN_PAGE_3_INFO_WORDS (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_MAN_3 {
MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
MPI2_CHIP_REVISION_ID ChipId; /*0x04 */
- U32
- Info[MPI2_MAN_PAGE_3_INFO_WORDS];/*0x08 */
+ U32 Info[]; /*0x08 */
} MPI2_CONFIG_PAGE_MAN_3,
*PTR_MPI2_CONFIG_PAGE_MAN_3,
Mpi2ManufacturingPage3_t,
@@ -765,12 +757,9 @@ typedef struct _MPI2_CONFIG_PAGE_MAN_4 {
/*Manufacturing Page 5 */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhys at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhys at runtime before using Phy[].
*/
-#ifndef MPI2_MAN_PAGE_5_PHY_ENTRIES
-#define MPI2_MAN_PAGE_5_PHY_ENTRIES (1)
-#endif
typedef struct _MPI2_MANUFACTURING5_ENTRY {
U64 WWID; /*0x00 */
@@ -787,8 +776,7 @@ typedef struct _MPI2_CONFIG_PAGE_MAN_5 {
U16 Reserved2; /*0x06 */
U32 Reserved3; /*0x08 */
U32 Reserved4; /*0x0C */
- MPI2_MANUFACTURING5_ENTRY
- Phy[MPI2_MAN_PAGE_5_PHY_ENTRIES];/*0x08 */
+ MPI2_MANUFACTURING5_ENTRY Phy[]; /*0x10 */
} MPI2_CONFIG_PAGE_MAN_5,
*PTR_MPI2_CONFIG_PAGE_MAN_5,
Mpi2ManufacturingPage5_t,
@@ -864,12 +852,9 @@ typedef struct _MPI2_MANPAGE7_CONNECTOR_INFO {
#define MPI2_MANPAGE7_SLOT_UNKNOWN (0xFFFF)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhys at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhys at runtime before using ConnectorInfo[].
*/
-#ifndef MPI2_MANPAGE7_CONNECTOR_INFO_MAX
-#define MPI2_MANPAGE7_CONNECTOR_INFO_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_MAN_7 {
MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
@@ -880,8 +865,7 @@ typedef struct _MPI2_CONFIG_PAGE_MAN_7 {
U8 NumPhys; /*0x20 */
U8 Reserved3; /*0x21 */
U16 Reserved4; /*0x22 */
- MPI2_MANPAGE7_CONNECTOR_INFO
- ConnectorInfo[MPI2_MANPAGE7_CONNECTOR_INFO_MAX]; /*0x24 */
+ MPI2_MANPAGE7_CONNECTOR_INFO ConnectorInfo[]; /*0x24 */
} MPI2_CONFIG_PAGE_MAN_7,
*PTR_MPI2_CONFIG_PAGE_MAN_7,
Mpi2ManufacturingPage7_t,
@@ -1019,12 +1003,9 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_3 {
/*IO Unit Page 5 */
/*
- *Upper layer code (drivers, utilities, etc.) should leave this define set to
- *one and check the value returned for NumDmaEngines at runtime.
+ *Upper layer code (drivers, utilities, etc.) should check the value returned
+ *for NumDmaEngines at runtime before using DmaEngineCapabilities[].
*/
-#ifndef MPI2_IOUNITPAGE5_DMAENGINE_ENTRIES
-#define MPI2_IOUNITPAGE5_DMAENGINE_ENTRIES (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_5 {
MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
@@ -1042,7 +1023,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_5 {
U32 Reserved2; /*0x24 */
U32 Reserved3; /*0x28 */
U32
- DmaEngineCapabilities[MPI2_IOUNITPAGE5_DMAENGINE_ENTRIES]; /*0x2C */
+ DmaEngineCapabilities[]; /*0x2C */
} MPI2_CONFIG_PAGE_IO_UNIT_5,
*PTR_MPI2_CONFIG_PAGE_IO_UNIT_5,
Mpi2IOUnitPage5_t, *pMpi2IOUnitPage5_t;
@@ -1259,12 +1240,9 @@ typedef struct _MPI2_IOUNIT9_SENSOR {
#define MPI2_IOUNIT9_SENSOR_FLAGS_TEMP_VALID (0x01)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumSensors at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumSensors at runtime before using Sensor[].
*/
-#ifndef MPI2_IOUNITPAGE9_SENSOR_ENTRIES
-#define MPI2_IOUNITPAGE9_SENSOR_ENTRIES (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_9 {
MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
@@ -1273,8 +1251,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_9 {
U8 NumSensors; /*0x0C */
U8 Reserved4; /*0x0D */
U16 Reserved3; /*0x0E */
- MPI2_IOUNIT9_SENSOR
- Sensor[MPI2_IOUNITPAGE9_SENSOR_ENTRIES];/*0x10 */
+ MPI2_IOUNIT9_SENSOR Sensor[]; /*0x10 */
} MPI2_CONFIG_PAGE_IO_UNIT_9,
*PTR_MPI2_CONFIG_PAGE_IO_UNIT_9,
Mpi2IOUnitPage9_t, *pMpi2IOUnitPage9_t;
@@ -1294,12 +1271,9 @@ typedef struct _MPI2_IOUNIT10_FUNCTION {
*pMpi2IOUnit10Function_t;
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumFunctions at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumFunctions at runtime before using Function[].
*/
-#ifndef MPI2_IOUNITPAGE10_FUNCTION_ENTRIES
-#define MPI2_IOUNITPAGE10_FUNCTION_ENTRIES (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_10 {
MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
@@ -1308,8 +1282,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_10 {
U16 Reserved2; /*0x06 */
U32 Reserved3; /*0x08 */
U32 Reserved4; /*0x0C */
- MPI2_IOUNIT10_FUNCTION
- Function[MPI2_IOUNITPAGE10_FUNCTION_ENTRIES];/*0x10 */
+ MPI2_IOUNIT10_FUNCTION Function[]; /*0x10 */
} MPI2_CONFIG_PAGE_IO_UNIT_10,
*PTR_MPI2_CONFIG_PAGE_IO_UNIT_10,
Mpi2IOUnitPage10_t, *pMpi2IOUnitPage10_t;
@@ -1764,12 +1737,9 @@ typedef struct _MPI2_CONFIG_PAGE_BIOS_3 {
/*BIOS Page 4 */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhys at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhys at runtime before using Phy[].
*/
-#ifndef MPI2_BIOS_PAGE_4_PHY_ENTRIES
-#define MPI2_BIOS_PAGE_4_PHY_ENTRIES (1)
-#endif
typedef struct _MPI2_BIOS4_ENTRY {
U64 ReassignmentWWID; /*0x00 */
@@ -1782,8 +1752,7 @@ typedef struct _MPI2_CONFIG_PAGE_BIOS_4 {
U8 NumPhys; /*0x04 */
U8 Reserved1; /*0x05 */
U16 Reserved2; /*0x06 */
- MPI2_BIOS4_ENTRY
- Phy[MPI2_BIOS_PAGE_4_PHY_ENTRIES]; /*0x08 */
+ MPI2_BIOS4_ENTRY Phy[]; /*0x08 */
} MPI2_CONFIG_PAGE_BIOS_4, *PTR_MPI2_CONFIG_PAGE_BIOS_4,
Mpi2BiosPage4_t, *pMpi2BiosPage4_t;
@@ -2045,12 +2014,9 @@ typedef struct _MPI2_CONFIG_PAGE_RD_PDISK_0 {
/*RAID Physical Disk Page 1 */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhysDiskPaths at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhysDiskPaths at runtime before using PhysicalDiskPath[].
*/
-#ifndef MPI2_RAID_PHYS_DISK1_PATH_MAX
-#define MPI2_RAID_PHYS_DISK1_PATH_MAX (1)
-#endif
typedef struct _MPI2_RAIDPHYSDISK1_PATH {
U16 DevHandle; /*0x00 */
@@ -2075,8 +2041,7 @@ typedef struct _MPI2_CONFIG_PAGE_RD_PDISK_1 {
U8 PhysDiskNum; /*0x05 */
U16 Reserved1; /*0x06 */
U32 Reserved2; /*0x08 */
- MPI2_RAIDPHYSDISK1_PATH
- PhysicalDiskPath[MPI2_RAID_PHYS_DISK1_PATH_MAX];/*0x0C */
+ MPI2_RAIDPHYSDISK1_PATH PhysicalDiskPath[]; /*0x0C */
} MPI2_CONFIG_PAGE_RD_PDISK_1,
*PTR_MPI2_CONFIG_PAGE_RD_PDISK_1,
Mpi2RaidPhysDiskPage1_t,
@@ -2502,12 +2467,9 @@ typedef struct _MPI2_SAS_IO_UNIT5_PHY_PM_SETTINGS {
#define MPI2_SASIOUNIT5_ITE_ONE_MICROSECOND (0)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhys at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhys at runtime before using SASPhyPowerManagementSettings[].
*/
-#ifndef MPI2_SAS_IOUNIT5_PHY_MAX
-#define MPI2_SAS_IOUNIT5_PHY_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_5 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -2516,7 +2478,7 @@ typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_5 {
U16 Reserved2;/*0x0A */
U32 Reserved3;/*0x0C */
MPI2_SAS_IO_UNIT5_PHY_PM_SETTINGS
- SASPhyPowerManagementSettings[MPI2_SAS_IOUNIT5_PHY_MAX];/*0x10 */
+ SASPhyPowerManagementSettings[]; /*0x10 */
} MPI2_CONFIG_PAGE_SASIOUNIT_5,
*PTR_MPI2_CONFIG_PAGE_SASIOUNIT_5,
Mpi2SasIOUnitPage5_t, *pMpi2SasIOUnitPage5_t;
@@ -2554,12 +2516,9 @@ typedef struct _MPI2_SAS_IO_UNIT6_PORT_WIDTH_MOD_GROUP_STATUS {
#define MPI2_SASIOUNIT6_MODULATION_100_PERCENT (0x03)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumGroups at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumGroups at runtime before using PortWidthModulationGroupStatus[].
*/
-#ifndef MPI2_SAS_IOUNIT6_GROUP_MAX
-#define MPI2_SAS_IOUNIT6_GROUP_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_6 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -2569,7 +2528,7 @@ typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_6 {
U8 Reserved3; /*0x11 */
U16 Reserved4; /*0x12 */
MPI2_SAS_IO_UNIT6_PORT_WIDTH_MOD_GROUP_STATUS
- PortWidthModulationGroupStatus[MPI2_SAS_IOUNIT6_GROUP_MAX]; /*0x14 */
+ PortWidthModulationGroupStatus[]; /*0x14 */
} MPI2_CONFIG_PAGE_SASIOUNIT_6,
*PTR_MPI2_CONFIG_PAGE_SASIOUNIT_6,
Mpi2SasIOUnitPage6_t, *pMpi2SasIOUnitPage6_t;
@@ -2597,12 +2556,9 @@ typedef struct _MPI2_SAS_IO_UNIT7_PORT_WIDTH_MOD_GROUP_SETTINGS {
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumGroups at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumGroups at runtime before using PortWidthModulationGroupSettings[].
*/
-#ifndef MPI2_SAS_IOUNIT7_GROUP_MAX
-#define MPI2_SAS_IOUNIT7_GROUP_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_7 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -2615,7 +2571,7 @@ typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_7 {
U8 Reserved4; /*0x15 */
U16 Reserved5; /*0x16 */
MPI2_SAS_IO_UNIT7_PORT_WIDTH_MOD_GROUP_SETTINGS
- PortWidthModulationGroupSettings[MPI2_SAS_IOUNIT7_GROUP_MAX];/*0x18 */
+ PortWidthModulationGroupSettings[]; /*0x18 */
} MPI2_CONFIG_PAGE_SASIOUNIT_7,
*PTR_MPI2_CONFIG_PAGE_SASIOUNIT_7,
Mpi2SasIOUnitPage7_t, *pMpi2SasIOUnitPage7_t;
@@ -3086,12 +3042,9 @@ typedef struct _MPI2_SASPHY2_PHY_EVENT {
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhyEvents at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhyEvents at runtime before using PhyEvent[].
*/
-#ifndef MPI2_SASPHY2_PHY_EVENT_MAX
-#define MPI2_SASPHY2_PHY_EVENT_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_SAS_PHY_2 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER
@@ -3105,7 +3058,7 @@ typedef struct _MPI2_CONFIG_PAGE_SAS_PHY_2 {
U16
Reserved3; /*0x0E */
MPI2_SASPHY2_PHY_EVENT
- PhyEvent[MPI2_SASPHY2_PHY_EVENT_MAX]; /*0x10 */
+ PhyEvent[]; /*0x10 */
} MPI2_CONFIG_PAGE_SAS_PHY_2,
*PTR_MPI2_CONFIG_PAGE_SAS_PHY_2,
Mpi2SasPhyPage2_t,
@@ -3200,12 +3153,9 @@ typedef struct _MPI2_SASPHY3_PHY_EVENT_CONFIG {
#define MPI2_SASPHY3_TFLAGS_EVENT_NOTIFY (0x0001)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhyEvents at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhyEvents at runtime before using PhyEventConfig[].
*/
-#ifndef MPI2_SASPHY3_PHY_EVENT_MAX
-#define MPI2_SASPHY3_PHY_EVENT_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_SAS_PHY_3 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER
@@ -3219,7 +3169,7 @@ typedef struct _MPI2_CONFIG_PAGE_SAS_PHY_3 {
U16
Reserved3; /*0x0E */
MPI2_SASPHY3_PHY_EVENT_CONFIG
- PhyEventConfig[MPI2_SASPHY3_PHY_EVENT_MAX]; /*0x10 */
+ PhyEventConfig[]; /*0x10 */
} MPI2_CONFIG_PAGE_SAS_PHY_3,
*PTR_MPI2_CONFIG_PAGE_SAS_PHY_3,
Mpi2SasPhyPage3_t, *pMpi2SasPhyPage3_t;
@@ -3358,12 +3308,9 @@ typedef struct _MPI2_CONFIG_PAGE_SAS_ENCLOSURE_0 {
/*Log Page 0 */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumLogEntries at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumLogEntries at runtime before using LogEntry[].
*/
-#ifndef MPI2_LOG_0_NUM_LOG_ENTRIES
-#define MPI2_LOG_0_NUM_LOG_ENTRIES (1)
-#endif
#define MPI2_LOG_0_LOG_DATA_LENGTH (0x1C)
@@ -3393,8 +3340,7 @@ typedef struct _MPI2_CONFIG_PAGE_LOG_0 {
U32 Reserved2; /*0x0C */
U16 NumLogEntries;/*0x10 */
U16 Reserved3; /*0x12 */
- MPI2_LOG_0_ENTRY
- LogEntry[MPI2_LOG_0_NUM_LOG_ENTRIES]; /*0x14 */
+ MPI2_LOG_0_ENTRY LogEntry[]; /*0x14 */
} MPI2_CONFIG_PAGE_LOG_0, *PTR_MPI2_CONFIG_PAGE_LOG_0,
Mpi2LogPage0_t, *pMpi2LogPage0_t;
@@ -3408,12 +3354,9 @@ typedef struct _MPI2_CONFIG_PAGE_LOG_0 {
/*RAID Page 0 */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumElements at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumElements at runtime before using ConfigElement[].
*/
-#ifndef MPI2_RAIDCONFIG0_MAX_ELEMENTS
-#define MPI2_RAIDCONFIG0_MAX_ELEMENTS (1)
-#endif
typedef struct _MPI2_RAIDCONFIG0_CONFIG_ELEMENT {
U16 ElementFlags; /*0x00 */
@@ -3446,8 +3389,7 @@ typedef struct _MPI2_CONFIG_PAGE_RAID_CONFIGURATION_0 {
U8 NumElements; /*0x2C */
U8 Reserved2; /*0x2D */
U16 Reserved3; /*0x2E */
- MPI2_RAIDCONFIG0_CONFIG_ELEMENT
- ConfigElement[MPI2_RAIDCONFIG0_MAX_ELEMENTS]; /*0x30 */
+ MPI2_RAIDCONFIG0_CONFIG_ELEMENT ConfigElement[];/*0x30 */
} MPI2_CONFIG_PAGE_RAID_CONFIGURATION_0,
*PTR_MPI2_CONFIG_PAGE_RAID_CONFIGURATION_0,
Mpi2RaidConfigurationPage0_t,
@@ -3687,12 +3629,9 @@ typedef struct _MPI26_PCIE_IO_UNIT0_PHY_DATA {
Mpi26PCIeIOUnit0PhyData_t, *pMpi26PCIeIOUnit0PhyData_t;
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhys at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhys at runtime before using PhyData[].
*/
-#ifndef MPI26_PCIE_IOUNIT0_PHY_MAX
-#define MPI26_PCIE_IOUNIT0_PHY_MAX (1)
-#endif
typedef struct _MPI26_CONFIG_PAGE_PIOUNIT_0 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -3701,7 +3640,7 @@ typedef struct _MPI26_CONFIG_PAGE_PIOUNIT_0 {
U8 InitStatus; /*0x0D */
U16 Reserved3; /*0x0E */
MPI26_PCIE_IO_UNIT0_PHY_DATA
- PhyData[MPI26_PCIE_IOUNIT0_PHY_MAX]; /*0x10 */
+ PhyData[]; /*0x10 */
} MPI26_CONFIG_PAGE_PIOUNIT_0,
*PTR_MPI26_CONFIG_PAGE_PIOUNIT_0,
Mpi26PCIeIOUnitPage0_t, *pMpi26PCIeIOUnitPage0_t;
@@ -3993,12 +3932,9 @@ typedef struct _MPI26_PCIELINK2_LINK_EVENT {
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumLinkEvents at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumLinkEvents at runtime before using LinkEvent[].
*/
-#ifndef MPI26_PCIELINK2_LINK_EVENT_MAX
-#define MPI26_PCIELINK2_LINK_EVENT_MAX (1)
-#endif
typedef struct _MPI26_CONFIG_PAGE_PCIELINK_2 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -4009,7 +3945,7 @@ typedef struct _MPI26_CONFIG_PAGE_PCIELINK_2 {
U8 Reserved3; /*0x0D */
U16 Reserved4; /*0x0E */
MPI26_PCIELINK2_LINK_EVENT
- LinkEvent[MPI26_PCIELINK2_LINK_EVENT_MAX]; /*0x10 */
+ LinkEvent[]; /*0x10 */
} MPI26_CONFIG_PAGE_PCIELINK_2, *PTR_MPI26_CONFIG_PAGE_PCIELINK_2,
Mpi26PcieLinkPage2_t, *pMpi26PcieLinkPage2_t;
@@ -4067,12 +4003,9 @@ typedef struct _MPI26_PCIELINK3_LINK_EVENT_CONFIG {
#define MPI26_PCIELINK3_TFLAGS_EVENT_NOTIFY (0x0001)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumLinkEvents at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumLinkEvents at runtime before using LinkEventConfig[].
*/
-#ifndef MPI26_PCIELINK3_LINK_EVENT_MAX
-#define MPI26_PCIELINK3_LINK_EVENT_MAX (1)
-#endif
typedef struct _MPI26_CONFIG_PAGE_PCIELINK_3 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -4083,7 +4016,7 @@ typedef struct _MPI26_CONFIG_PAGE_PCIELINK_3 {
U8 Reserved3; /*0x0D */
U16 Reserved4; /*0x0E */
MPI26_PCIELINK3_LINK_EVENT_CONFIG
- LinkEventConfig[MPI26_PCIELINK3_LINK_EVENT_MAX]; /*0x10 */
+ LinkEventConfig[]; /*0x10 */
} MPI26_CONFIG_PAGE_PCIELINK_3, *PTR_MPI26_CONFIG_PAGE_PCIELINK_3,
Mpi26PcieLinkPage3_t, *pMpi26PcieLinkPage3_t;
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_image.h b/drivers/scsi/mpt3sas/mpi/mpi2_image.h
index 33b9c3a6fd40..798ab6e33eb9 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_image.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_image.h
@@ -295,20 +295,9 @@ typedef struct _MPI2_EXT_IMAGE_HEADER {
/*FLASH Layout Extended Image Data */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check RegionsPerLayout at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check NumberOfLayouts and
+ *RegionsPerLayout at runtime before using Layout[] and Region[].
*/
-#ifndef MPI2_FLASH_NUMBER_OF_REGIONS
-#define MPI2_FLASH_NUMBER_OF_REGIONS (1)
-#endif
-
-/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check NumberOfLayouts at runtime.
- */
-#ifndef MPI2_FLASH_NUMBER_OF_LAYOUTS
-#define MPI2_FLASH_NUMBER_OF_LAYOUTS (1)
-#endif
typedef struct _MPI2_FLASH_REGION {
U8 RegionType; /*0x00 */
@@ -325,7 +314,7 @@ typedef struct _MPI2_FLASH_LAYOUT {
U32 Reserved1; /*0x04 */
U32 Reserved2; /*0x08 */
U32 Reserved3; /*0x0C */
- MPI2_FLASH_REGION Region[MPI2_FLASH_NUMBER_OF_REGIONS]; /*0x10 */
+ MPI2_FLASH_REGION Region[]; /*0x10 */
} MPI2_FLASH_LAYOUT, *PTR_MPI2_FLASH_LAYOUT,
Mpi2FlashLayout_t, *pMpi2FlashLayout_t;
@@ -339,7 +328,7 @@ typedef struct _MPI2_FLASH_LAYOUT_DATA {
U16 MinimumSectorAlignment; /*0x08 */
U16 Reserved3; /*0x0A */
U32 Reserved4; /*0x0C */
- MPI2_FLASH_LAYOUT Layout[MPI2_FLASH_NUMBER_OF_LAYOUTS]; /*0x10 */
+ MPI2_FLASH_LAYOUT Layout[]; /*0x10 */
} MPI2_FLASH_LAYOUT_DATA, *PTR_MPI2_FLASH_LAYOUT_DATA,
Mpi2FlashLayoutData_t, *pMpi2FlashLayoutData_t;
@@ -373,12 +362,9 @@ typedef struct _MPI2_FLASH_LAYOUT_DATA {
/*Supported Devices Extended Image Data */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check NumberOfDevices at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check NumberOfDevices at
+ *runtime before using SupportedDevice[].
*/
-#ifndef MPI2_SUPPORTED_DEVICES_IMAGE_NUM_DEVICES
-#define MPI2_SUPPORTED_DEVICES_IMAGE_NUM_DEVICES (1)
-#endif
typedef struct _MPI2_SUPPORTED_DEVICE {
U16 DeviceID; /*0x00 */
@@ -399,7 +385,7 @@ typedef struct _MPI2_SUPPORTED_DEVICES_DATA {
U8 Reserved2; /*0x03 */
U32 Reserved3; /*0x04 */
MPI2_SUPPORTED_DEVICE
- SupportedDevice[MPI2_SUPPORTED_DEVICES_IMAGE_NUM_DEVICES];/*0x08 */
+ SupportedDevice[]; /*0x08 */
} MPI2_SUPPORTED_DEVICES_DATA, *PTR_MPI2_SUPPORTED_DEVICES_DATA,
Mpi2SupportedDevicesData_t, *pMpi2SupportedDevicesData_t;
@@ -464,7 +450,7 @@ typedef struct _MPI25_ENCRYPTED_HASH_ENTRY {
U8 EncryptionAlgorithm; /*0x02 */
U8 Reserved1; /*0x03 */
U32 Reserved2; /*0x04 */
- U32 EncryptedHash[1]; /*0x08 */ /* variable length */
+ U32 EncryptedHash[]; /*0x08 */
} MPI25_ENCRYPTED_HASH_ENTRY, *PTR_MPI25_ENCRYPTED_HASH_ENTRY,
Mpi25EncryptedHashEntry_t, *pMpi25EncryptedHashEntry_t;
@@ -508,7 +494,7 @@ typedef struct _MPI25_ENCRYPTED_HASH_DATA {
U8 NumHash; /*0x01 */
U16 Reserved1; /*0x02 */
U32 Reserved2; /*0x04 */
- MPI25_ENCRYPTED_HASH_ENTRY EncryptedHashEntry[1]; /*0x08 */
+ MPI25_ENCRYPTED_HASH_ENTRY EncryptedHashEntry[]; /*0x08 */
} MPI25_ENCRYPTED_HASH_DATA, *PTR_MPI25_ENCRYPTED_HASH_DATA,
Mpi25EncryptedHashData_t, *pMpi25EncryptedHashData_t;
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h b/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
index 2c57115172cf..d92852591134 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
@@ -808,12 +808,9 @@ typedef struct _MPI2_EVENT_DATA_IR_PHYSICAL_DISK {
/*Integrated RAID Configuration Change List Event data */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check NumElements at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check NumElements at
+ *runtime before using ConfigElement[].
*/
-#ifndef MPI2_EVENT_IR_CONFIG_ELEMENT_COUNT
-#define MPI2_EVENT_IR_CONFIG_ELEMENT_COUNT (1)
-#endif
typedef struct _MPI2_EVENT_IR_CONFIG_ELEMENT {
U16 ElementFlags; /*0x00 */
@@ -848,7 +845,7 @@ typedef struct _MPI2_EVENT_DATA_IR_CONFIG_CHANGE_LIST {
U8 ConfigNum; /*0x03 */
U32 Flags; /*0x04 */
MPI2_EVENT_IR_CONFIG_ELEMENT
- ConfigElement[MPI2_EVENT_IR_CONFIG_ELEMENT_COUNT];/*0x08 */
+ ConfigElement[];/*0x08 */
} MPI2_EVENT_DATA_IR_CONFIG_CHANGE_LIST,
*PTR_MPI2_EVENT_DATA_IR_CONFIG_CHANGE_LIST,
Mpi2EventDataIrConfigChangeList_t,
@@ -969,12 +966,9 @@ typedef struct _MPI2_EVENT_DATA_SAS_INIT_TABLE_OVERFLOW {
/*SAS Topology Change List Event data */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check NumEntries at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check NumEntries at
+ *runtime before using PHY[].
*/
-#ifndef MPI2_EVENT_SAS_TOPO_PHY_COUNT
-#define MPI2_EVENT_SAS_TOPO_PHY_COUNT (1)
-#endif
typedef struct _MPI2_EVENT_SAS_TOPO_PHY_ENTRY {
U16 AttachedDevHandle; /*0x00 */
@@ -994,7 +988,7 @@ typedef struct _MPI2_EVENT_DATA_SAS_TOPOLOGY_CHANGE_LIST {
U8 ExpStatus; /*0x0A */
U8 PhysicalPort; /*0x0B */
MPI2_EVENT_SAS_TOPO_PHY_ENTRY
- PHY[MPI2_EVENT_SAS_TOPO_PHY_COUNT]; /*0x0C */
+ PHY[]; /*0x0C */
} MPI2_EVENT_DATA_SAS_TOPOLOGY_CHANGE_LIST,
*PTR_MPI2_EVENT_DATA_SAS_TOPOLOGY_CHANGE_LIST,
Mpi2EventDataSasTopologyChangeList_t,
@@ -1229,12 +1223,9 @@ typedef struct _MPI26_EVENT_DATA_PCIE_ENUMERATION {
/*PCIe Topology Change List Event data (MPI v2.6 and later) */
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check NumEntries at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check NumEntries at
+ *runtime before using PortEntry[].
*/
-#ifndef MPI26_EVENT_PCIE_TOPO_PORT_COUNT
-#define MPI26_EVENT_PCIE_TOPO_PORT_COUNT (1)
-#endif
typedef struct _MPI26_EVENT_PCIE_TOPO_PORT_ENTRY {
U16 AttachedDevHandle; /*0x00 */
@@ -1286,7 +1277,7 @@ typedef struct _MPI26_EVENT_DATA_PCIE_TOPOLOGY_CHANGE_LIST {
U8 SwitchStatus; /*0x0A */
U8 PhysicalPort; /*0x0B */
MPI26_EVENT_PCIE_TOPO_PORT_ENTRY
- PortEntry[MPI26_EVENT_PCIE_TOPO_PORT_COUNT]; /*0x0C */
+ PortEntry[]; /*0x0C */
} MPI26_EVENT_DATA_PCIE_TOPOLOGY_CHANGE_LIST,
*PTR_MPI26_EVENT_DATA_PCIE_TOPOLOGY_CHANGE_LIST,
Mpi26EventDataPCIeTopologyChangeList_t,
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/6] scsi: mpt3sas: Use flexible arrays when less obviously possible
2023-07-25 16:13 [PATCH 0/6] scsi: mpt3sas: Use flexible arrays and do a few cleanups James Seo
2023-07-25 16:13 ` [PATCH 1/6] scsi: mpt3sas: Use flexible arrays when obviously possible James Seo
@ 2023-07-25 16:13 ` James Seo
2023-07-28 22:26 ` Kees Cook
2023-07-25 16:13 ` [PATCH 3/6] scsi: mpt3sas: Use struct_size() for struct size calculations James Seo
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: James Seo @ 2023-07-25 16:13 UTC (permalink / raw)
To: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani
Cc: James Seo, James E.J. Bottomley, Martin K. Petersen, Kees Cook,
Gustavo A. R. Silva, MPT-FusionLinux.pdl, linux-scsi,
linux-kernel
These old-style 1-length variable arrays can be directly converted
into C99 flexible array members without any further source changes
and without any meaningful binary changes. All uses of the affected
structs were investigated, and the existing code somehow manages to
weather the reduced sizeof() the affected structs in every case.
For example, we may have previously requested 26 bytes from the
device to fill a 26-byte buffer for a struct, and we are now dealing
with 12 bytes due to sizeof() reduction. However, either we never use
the variable array anyway, or we follow up with a subsequent request
for the same struct in its entirety after using its variable element
count (usually taken from one of the struct fields) to allocate a
larger buffer.
It also turns out that size calculations are always performed as e.g.
"offsetof(struct foo, arr_of_bar) + n * sizeof(bar)" instead of
"sizeof(struct foo) + (n - 1) * sizeof(bar)", and are therefore
already correct.
Signed-off-by: James Seo <james@equiv.tech>
---
drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 48 +++++++++-------------------
1 file changed, 15 insertions(+), 33 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
index 42d820159c44..f07215fbc140 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
@@ -1200,12 +1200,9 @@ typedef struct _MPI2_IOUNIT8_SENSOR {
#define MPI2_IOUNIT8_SENSOR_FLAGS_T0_ENABLE (0x0001)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumSensors at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumSensors at runtime before using Sensor[].
*/
-#ifndef MPI2_IOUNITPAGE8_SENSOR_ENTRIES
-#define MPI2_IOUNITPAGE8_SENSOR_ENTRIES (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_8 {
MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
@@ -1214,8 +1211,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_8 {
U8 NumSensors; /*0x0C */
U8 PollingInterval; /*0x0D */
U16 Reserved3; /*0x0E */
- MPI2_IOUNIT8_SENSOR
- Sensor[MPI2_IOUNITPAGE8_SENSOR_ENTRIES];/*0x10 */
+ MPI2_IOUNIT8_SENSOR Sensor[]; /*0x10 */
} MPI2_CONFIG_PAGE_IO_UNIT_8,
*PTR_MPI2_CONFIG_PAGE_IO_UNIT_8,
Mpi2IOUnitPage8_t, *pMpi2IOUnitPage8_t;
@@ -1805,12 +1801,9 @@ typedef struct _MPI2_RAIDVOL0_SETTINGS {
#define MPI2_RAIDVOL0_SETTING_ENABLE_WRITE_CACHING (0x0002)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhysDisks at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhysDisks at runtime before using PhysDisk[].
*/
-#ifndef MPI2_RAID_VOL_PAGE_0_PHYSDISK_MAX
-#define MPI2_RAID_VOL_PAGE_0_PHYSDISK_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_RAID_VOL_0 {
MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
@@ -1830,8 +1823,7 @@ typedef struct _MPI2_CONFIG_PAGE_RAID_VOL_0 {
U8 Reserved2; /*0x25 */
U8 Reserved3; /*0x26 */
U8 InactiveStatus; /*0x27 */
- MPI2_RAIDVOL0_PHYS_DISK
- PhysDisk[MPI2_RAID_VOL_PAGE_0_PHYSDISK_MAX]; /*0x28 */
+ MPI2_RAIDVOL0_PHYS_DISK PhysDisk[]; /*0x28 */
} MPI2_CONFIG_PAGE_RAID_VOL_0,
*PTR_MPI2_CONFIG_PAGE_RAID_VOL_0,
Mpi2RaidVolPage0_t, *pMpi2RaidVolPage0_t;
@@ -2186,12 +2178,9 @@ typedef struct _MPI2_SAS_IO_UNIT0_PHY_DATA {
*pMpi2SasIOUnit0PhyData_t;
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhys at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhys at runtime before using PhyData[].
*/
-#ifndef MPI2_SAS_IOUNIT0_PHY_MAX
-#define MPI2_SAS_IOUNIT0_PHY_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_0 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -2199,8 +2188,7 @@ typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_0 {
U8 NumPhys; /*0x0C */
U8 Reserved2;/*0x0D */
U16 Reserved3;/*0x0E */
- MPI2_SAS_IO_UNIT0_PHY_DATA
- PhyData[MPI2_SAS_IOUNIT0_PHY_MAX]; /*0x10 */
+ MPI2_SAS_IO_UNIT0_PHY_DATA PhyData[];/*0x10 */
} MPI2_CONFIG_PAGE_SASIOUNIT_0,
*PTR_MPI2_CONFIG_PAGE_SASIOUNIT_0,
Mpi2SasIOUnitPage0_t, *pMpi2SasIOUnitPage0_t;
@@ -2261,12 +2249,9 @@ typedef struct _MPI2_SAS_IO_UNIT1_PHY_DATA {
*pMpi2SasIOUnit1PhyData_t;
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhys at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhys at runtime before using PhyData[].
*/
-#ifndef MPI2_SAS_IOUNIT1_PHY_MAX
-#define MPI2_SAS_IOUNIT1_PHY_MAX (1)
-#endif
typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_1 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -2287,7 +2272,7 @@ typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_1 {
U8
IODeviceMissingDelay; /*0x13 */
MPI2_SAS_IO_UNIT1_PHY_DATA
- PhyData[MPI2_SAS_IOUNIT1_PHY_MAX]; /*0x14 */
+ PhyData[]; /*0x14 */
} MPI2_CONFIG_PAGE_SASIOUNIT_1,
*PTR_MPI2_CONFIG_PAGE_SASIOUNIT_1,
Mpi2SasIOUnitPage1_t, *pMpi2SasIOUnitPage1_t;
@@ -3683,12 +3668,9 @@ typedef struct _MPI26_PCIE_IO_UNIT1_PHY_DATA {
#define MPI26_PCIEIOUNIT1_LINKFLAGS_SRNS_EN (0x02)
/*
- *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for NumPhys at runtime.
+ *Host code (drivers, BIOS, utilities, etc.) should check the value returned
+ *for NumPhys at runtime before using PhyData[].
*/
-#ifndef MPI26_PCIE_IOUNIT1_PHY_MAX
-#define MPI26_PCIE_IOUNIT1_PHY_MAX (1)
-#endif
typedef struct _MPI26_CONFIG_PAGE_PIOUNIT_1 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
@@ -3700,7 +3682,7 @@ typedef struct _MPI26_CONFIG_PAGE_PIOUNIT_1 {
U8 DMDReportPCIe; /*0x11 */
U16 Reserved2; /*0x12 */
MPI26_PCIE_IO_UNIT1_PHY_DATA
- PhyData[MPI26_PCIE_IOUNIT1_PHY_MAX];/*0x14 */
+ PhyData[]; /*0x14 */
} MPI26_CONFIG_PAGE_PIOUNIT_1,
*PTR_MPI26_CONFIG_PAGE_PIOUNIT_1,
Mpi26PCIeIOUnitPage1_t, *pMpi26PCIeIOUnitPage1_t;
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/6] scsi: mpt3sas: Use struct_size() for struct size calculations
2023-07-25 16:13 [PATCH 0/6] scsi: mpt3sas: Use flexible arrays and do a few cleanups James Seo
2023-07-25 16:13 ` [PATCH 1/6] scsi: mpt3sas: Use flexible arrays when obviously possible James Seo
2023-07-25 16:13 ` [PATCH 2/6] scsi: mpt3sas: Use flexible arrays when less " James Seo
@ 2023-07-25 16:13 ` James Seo
2023-07-25 16:13 ` [PATCH 4/6] scsi: mpt3sas: Fix an outdated comment James Seo
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: James Seo @ 2023-07-25 16:13 UTC (permalink / raw)
To: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani
Cc: James Seo, James E.J. Bottomley, Martin K. Petersen, Kees Cook,
Gustavo A. R. Silva, MPT-FusionLinux.pdl, linux-scsi,
linux-kernel
After converting variable-length arrays to flexible array members,
use the bounds-checking struct_size() helper when possible to avoid
open-coded arithmetic struct size calculations.
Signed-off-by: James Seo <james@equiv.tech>
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 3 +--
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 18 ++++++------------
drivers/scsi/mpt3sas/mpt3sas_transport.c | 9 +++------
drivers/scsi/mpt3sas/mpt3sas_warpdrive.c | 3 +--
4 files changed, 11 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 53f5492579cb..2ae0185938f3 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4876,8 +4876,7 @@ mpt3sas_base_update_missing_delay(struct MPT3SAS_ADAPTER *ioc,
if (!num_phys)
return;
- sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData) + (num_phys *
- sizeof(Mpi2SasIOUnit1PhyData_t));
+ sz = struct_size(sas_iounit_pg1, PhyData, num_phys);
sas_iounit_pg1 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg1) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index c3c1f466fe01..d5426a520a77 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -2431,8 +2431,7 @@ _scsih_get_volume_capabilities(struct MPT3SAS_ADAPTER *ioc,
}
raid_device->num_pds = num_pds;
- sz = offsetof(Mpi2RaidVolPage0_t, PhysDisk) + (num_pds *
- sizeof(Mpi2RaidVol0PhysDisk_t));
+ sz = struct_size(vol_pg0, PhysDisk, num_pds);
vol_pg0 = kzalloc(sz, GFP_KERNEL);
if (!vol_pg0) {
dfailprintk(ioc,
@@ -5966,8 +5965,7 @@ _scsih_update_vphys_after_reset(struct MPT3SAS_ADAPTER *ioc)
/*
* Read SASIOUnitPage0 to get each HBA Phy's data.
*/
- sz = offsetof(Mpi2SasIOUnitPage0_t, PhyData) +
- (ioc->sas_hba.num_phys * sizeof(Mpi2SasIOUnit0PhyData_t));
+ sz = struct_size(sas_iounit_pg0, PhyData, ioc->sas_hba.num_phys);
sas_iounit_pg0 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg0) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
@@ -6145,8 +6143,7 @@ _scsih_get_port_table_after_reset(struct MPT3SAS_ADAPTER *ioc,
u64 attached_sas_addr;
u8 found = 0, port_count = 0, port_id;
- sz = offsetof(Mpi2SasIOUnitPage0_t, PhyData) + (ioc->sas_hba.num_phys
- * sizeof(Mpi2SasIOUnit0PhyData_t));
+ sz = struct_size(sas_iounit_pg0, PhyData, ioc->sas_hba.num_phys);
sas_iounit_pg0 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg0) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
@@ -6579,8 +6576,7 @@ _scsih_sas_host_refresh(struct MPT3SAS_ADAPTER *ioc)
ioc_info(ioc, "updating handles for sas_host(0x%016llx)\n",
(u64)ioc->sas_hba.sas_address));
- sz = offsetof(Mpi2SasIOUnitPage0_t, PhyData) + (ioc->sas_hba.num_phys
- * sizeof(Mpi2SasIOUnit0PhyData_t));
+ sz = struct_size(sas_iounit_pg0, PhyData, ioc->sas_hba.num_phys);
sas_iounit_pg0 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg0) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
@@ -6731,8 +6727,7 @@ _scsih_sas_host_add(struct MPT3SAS_ADAPTER *ioc)
ioc->sas_hba.num_phys = num_phys;
/* sas_iounit page 0 */
- sz = offsetof(Mpi2SasIOUnitPage0_t, PhyData) + (ioc->sas_hba.num_phys *
- sizeof(Mpi2SasIOUnit0PhyData_t));
+ sz = struct_size(sas_iounit_pg0, PhyData, ioc->sas_hba.num_phys);
sas_iounit_pg0 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg0) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
@@ -6754,8 +6749,7 @@ _scsih_sas_host_add(struct MPT3SAS_ADAPTER *ioc)
}
/* sas_iounit page 1 */
- sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData) + (ioc->sas_hba.num_phys *
- sizeof(Mpi2SasIOUnit1PhyData_t));
+ sz = struct_size(sas_iounit_pg1, PhyData, ioc->sas_hba.num_phys);
sas_iounit_pg1 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg1) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c b/drivers/scsi/mpt3sas/mpt3sas_transport.c
index e8a4750f6ec4..421ea511b664 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c
@@ -1792,8 +1792,7 @@ _transport_phy_enable(struct sas_phy *phy, int enable)
/* handle hba phys */
/* read sas_iounit page 0 */
- sz = offsetof(Mpi2SasIOUnitPage0_t, PhyData) + (ioc->sas_hba.num_phys *
- sizeof(Mpi2SasIOUnit0PhyData_t));
+ sz = struct_size(sas_iounit_pg0, PhyData, ioc->sas_hba.num_phys);
sas_iounit_pg0 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg0) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
@@ -1833,8 +1832,7 @@ _transport_phy_enable(struct sas_phy *phy, int enable)
}
/* read sas_iounit page 1 */
- sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData) + (ioc->sas_hba.num_phys *
- sizeof(Mpi2SasIOUnit1PhyData_t));
+ sz = struct_size(sas_iounit_pg1, PhyData, ioc->sas_hba.num_phys);
sas_iounit_pg1 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg1) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
@@ -1944,8 +1942,7 @@ _transport_phy_speed(struct sas_phy *phy, struct sas_phy_linkrates *rates)
/* handle hba phys */
/* sas_iounit page 1 */
- sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData) + (ioc->sas_hba.num_phys *
- sizeof(Mpi2SasIOUnit1PhyData_t));
+ sz = struct_size(sas_iounit_pg1, PhyData, ioc->sas_hba.num_phys);
sas_iounit_pg1 = kzalloc(sz, GFP_KERNEL);
if (!sas_iounit_pg1) {
ioc_err(ioc, "failure at %s:%d/%s()!\n",
diff --git a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
index cc07ba41f507..1d64e5056a8a 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_warpdrive.c
@@ -141,8 +141,7 @@ mpt3sas_init_warpdrive_properties(struct MPT3SAS_ADAPTER *ioc,
return;
}
- sz = offsetof(Mpi2RaidVolPage0_t, PhysDisk) + (num_pds *
- sizeof(Mpi2RaidVol0PhysDisk_t));
+ sz = struct_size(vol_pg0, PhysDisk, num_pds);
vol_pg0 = kzalloc(sz, GFP_KERNEL);
if (!vol_pg0) {
ioc_info(ioc, "WarpDrive : Direct IO is disabled Memory allocation failure for RVPG0\n");
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/6] scsi: mpt3sas: Fix an outdated comment
2023-07-25 16:13 [PATCH 0/6] scsi: mpt3sas: Use flexible arrays and do a few cleanups James Seo
` (2 preceding siblings ...)
2023-07-25 16:13 ` [PATCH 3/6] scsi: mpt3sas: Use struct_size() for struct size calculations James Seo
@ 2023-07-25 16:13 ` James Seo
2023-07-28 22:27 ` Kees Cook
2023-07-25 16:13 ` [PATCH 5/6] scsi: mpt3sas: Fix typo of "TRIGGER" James Seo
2023-07-25 16:13 ` [PATCH 6/6] scsi: mpt3sas: Replace a dynamic allocation with a local variable James Seo
5 siblings, 1 reply; 14+ messages in thread
From: James Seo @ 2023-07-25 16:13 UTC (permalink / raw)
To: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani
Cc: James Seo, James E.J. Bottomley, Martin K. Petersen, Kees Cook,
Gustavo A. R. Silva, MPT-FusionLinux.pdl, linux-scsi,
linux-kernel
May reduce confusion for users of MPI2_CONFIG_PAGE_IO_UNIT_3::GPIOVal[].
Fixes: a1c4d7741323 ("scsi: mpt3sas: Replace unnecessary dynamic allocation with a static one")
Signed-off-by: James Seo <james@equiv.tech>
---
drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
index f07215fbc140..62bde43f64a8 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
@@ -974,7 +974,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_1 {
/*
*Host code (drivers, BIOS, utilities, etc.) should leave this define set to
- *one and check the value returned for GPIOCount at runtime.
+ *36 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 (36)
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/6] scsi: mpt3sas: Fix typo of "TRIGGER"
2023-07-25 16:13 [PATCH 0/6] scsi: mpt3sas: Use flexible arrays and do a few cleanups James Seo
` (3 preceding siblings ...)
2023-07-25 16:13 ` [PATCH 4/6] scsi: mpt3sas: Fix an outdated comment James Seo
@ 2023-07-25 16:13 ` James Seo
2023-07-28 22:27 ` Kees Cook
2023-07-25 16:13 ` [PATCH 6/6] scsi: mpt3sas: Replace a dynamic allocation with a local variable James Seo
5 siblings, 1 reply; 14+ messages in thread
From: James Seo @ 2023-07-25 16:13 UTC (permalink / raw)
To: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani
Cc: James Seo, James E.J. Bottomley, Martin K. Petersen, Kees Cook,
Gustavo A. R. Silva, MPT-FusionLinux.pdl, linux-scsi,
linux-kernel
Change "TIGGER" to "TRIGGER" in struct names and typedefs.
Signed-off-by: James Seo <james@equiv.tech>
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 6 +--
drivers/scsi/mpt3sas/mpt3sas_config.c | 6 +--
drivers/scsi/mpt3sas/mpt3sas_trigger_pages.h | 44 ++++++++++----------
3 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 2ae0185938f3..cd6f36094159 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -5026,7 +5026,7 @@ _base_get_event_diag_triggers(struct MPT3SAS_ADAPTER *ioc)
{
Mpi26DriverTriggerPage2_t trigger_pg2;
struct SL_WH_EVENT_TRIGGER_T *event_tg;
- MPI26_DRIVER_MPI_EVENT_TIGGER_ENTRY *mpi_event_tg;
+ MPI26_DRIVER_MPI_EVENT_TRIGGER_ENTRY *mpi_event_tg;
Mpi2ConfigReply_t mpi_reply;
int r = 0, i = 0;
u16 count = 0;
@@ -5078,7 +5078,7 @@ _base_get_scsi_diag_triggers(struct MPT3SAS_ADAPTER *ioc)
{
Mpi26DriverTriggerPage3_t trigger_pg3;
struct SL_WH_SCSI_TRIGGER_T *scsi_tg;
- MPI26_DRIVER_SCSI_SENSE_TIGGER_ENTRY *mpi_scsi_tg;
+ MPI26_DRIVER_SCSI_SENSE_TRIGGER_ENTRY *mpi_scsi_tg;
Mpi2ConfigReply_t mpi_reply;
int r = 0, i = 0;
u16 count = 0;
@@ -5130,7 +5130,7 @@ _base_get_mpi_diag_triggers(struct MPT3SAS_ADAPTER *ioc)
{
Mpi26DriverTriggerPage4_t trigger_pg4;
struct SL_WH_MPI_TRIGGER_T *status_tg;
- MPI26_DRIVER_IOCSTATUS_LOGINFO_TIGGER_ENTRY *mpi_status_tg;
+ MPI26_DRIVER_IOCSTATUS_LOGINFO_TRIGGER_ENTRY *mpi_status_tg;
Mpi2ConfigReply_t mpi_reply;
int r = 0, i = 0;
u16 count = 0;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_config.c b/drivers/scsi/mpt3sas/mpt3sas_config.c
index d114ef381c44..2e88f456fc34 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_config.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_config.c
@@ -2334,7 +2334,7 @@ mpt3sas_config_update_driver_trigger_pg2(struct MPT3SAS_ADAPTER *ioc,
tg_pg2.NumMPIEventTrigger = 0;
memset(&tg_pg2.MPIEventTriggers[0], 0,
NUM_VALID_ENTRIES * sizeof(
- MPI26_DRIVER_MPI_EVENT_TIGGER_ENTRY));
+ MPI26_DRIVER_MPI_EVENT_TRIGGER_ENTRY));
}
rc = _config_set_driver_trigger_pg2(ioc, &mpi_reply, &tg_pg2);
@@ -2493,7 +2493,7 @@ mpt3sas_config_update_driver_trigger_pg3(struct MPT3SAS_ADAPTER *ioc,
tg_pg3.NumSCSISenseTrigger = 0;
memset(&tg_pg3.SCSISenseTriggers[0], 0,
NUM_VALID_ENTRIES * sizeof(
- MPI26_DRIVER_SCSI_SENSE_TIGGER_ENTRY));
+ MPI26_DRIVER_SCSI_SENSE_TRIGGER_ENTRY));
}
rc = _config_set_driver_trigger_pg3(ioc, &mpi_reply, &tg_pg3);
@@ -2649,7 +2649,7 @@ mpt3sas_config_update_driver_trigger_pg4(struct MPT3SAS_ADAPTER *ioc,
tg_pg4.NumIOCStatusLogInfoTrigger = 0;
memset(&tg_pg4.IOCStatusLoginfoTriggers[0], 0,
NUM_VALID_ENTRIES * sizeof(
- MPI26_DRIVER_IOCSTATUS_LOGINFO_TIGGER_ENTRY));
+ MPI26_DRIVER_IOCSTATUS_LOGINFO_TRIGGER_ENTRY));
}
rc = _config_set_driver_trigger_pg4(ioc, &mpi_reply, &tg_pg4);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_trigger_pages.h b/drivers/scsi/mpt3sas/mpt3sas_trigger_pages.h
index 5f3328f011a2..edb8fe709089 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_trigger_pages.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_trigger_pages.h
@@ -20,12 +20,12 @@
#define MPI2_CONFIG_EXTPAGETYPE_DRIVER_PERSISTENT_TRIGGER (0xE0)
#define MPI26_DRIVER_TRIGGER_PAGE0_PAGEVERSION (0x01)
-typedef struct _MPI26_CONFIG_PAGE_DRIVER_TIGGER_0 {
+typedef struct _MPI26_CONFIG_PAGE_DRIVER_TRIGGER_0 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /* 0x00 */
U16 TriggerFlags; /* 0x08 */
U16 Reserved0xA; /* 0x0A */
U32 Reserved0xC[61]; /* 0x0C */
-} _MPI26_CONFIG_PAGE_DRIVER_TIGGER_0, Mpi26DriverTriggerPage0_t;
+} _MPI26_CONFIG_PAGE_DRIVER_TRIGGER_0, Mpi26DriverTriggerPage0_t;
/* Trigger Flags */
#define MPI26_DRIVER_TRIGGER0_FLAG_MASTER_TRIGGER_VALID (0x0001)
@@ -34,61 +34,61 @@ typedef struct _MPI26_CONFIG_PAGE_DRIVER_TIGGER_0 {
#define MPI26_DRIVER_TRIGGER0_FLAG_LOGINFO_TRIGGER_VALID (0x0008)
#define MPI26_DRIVER_TRIGGER_PAGE1_PAGEVERSION (0x01)
-typedef struct _MPI26_DRIVER_MASTER_TIGGER_ENTRY {
+typedef struct _MPI26_DRIVER_MASTER_TRIGGER_ENTRY {
U32 MasterTriggerFlags;
-} MPI26_DRIVER_MASTER_TIGGER_ENTRY;
+} MPI26_DRIVER_MASTER_TRIGGER_ENTRY;
#define MPI26_MAX_MASTER_TRIGGERS (1)
-typedef struct _MPI26_CONFIG_PAGE_DRIVER_TIGGER_1 {
+typedef struct _MPI26_CONFIG_PAGE_DRIVER_TRIGGER_1 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /* 0x00 */
U16 NumMasterTrigger; /* 0x08 */
U16 Reserved0xA; /* 0x0A */
- MPI26_DRIVER_MASTER_TIGGER_ENTRY MasterTriggers[MPI26_MAX_MASTER_TRIGGERS]; /* 0x0C */
-} MPI26_CONFIG_PAGE_DRIVER_TIGGER_1, Mpi26DriverTriggerPage1_t;
+ MPI26_DRIVER_MASTER_TRIGGER_ENTRY MasterTriggers[MPI26_MAX_MASTER_TRIGGERS]; /* 0x0C */
+} MPI26_CONFIG_PAGE_DRIVER_TRIGGER_1, Mpi26DriverTriggerPage1_t;
#define MPI26_DRIVER_TRIGGER_PAGE2_PAGEVERSION (0x01)
-typedef struct _MPI26_DRIVER_MPI_EVENT_TIGGER_ENTRY {
+typedef struct _MPI26_DRIVER_MPI_EVENT_TRIGGER_ENTRY {
U16 MPIEventCode; /* 0x00 */
U16 MPIEventCodeSpecific; /* 0x02 */
-} MPI26_DRIVER_MPI_EVENT_TIGGER_ENTRY;
+} MPI26_DRIVER_MPI_EVENT_TRIGGER_ENTRY;
#define MPI26_MAX_MPI_EVENT_TRIGGERS (20)
-typedef struct _MPI26_CONFIG_PAGE_DRIVER_TIGGER_2 {
+typedef struct _MPI26_CONFIG_PAGE_DRIVER_TRIGGER_2 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /* 0x00 */
U16 NumMPIEventTrigger; /* 0x08 */
U16 Reserved0xA; /* 0x0A */
- MPI26_DRIVER_MPI_EVENT_TIGGER_ENTRY MPIEventTriggers[MPI26_MAX_MPI_EVENT_TRIGGERS]; /* 0x0C */
-} MPI26_CONFIG_PAGE_DRIVER_TIGGER_2, Mpi26DriverTriggerPage2_t;
+ MPI26_DRIVER_MPI_EVENT_TRIGGER_ENTRY MPIEventTriggers[MPI26_MAX_MPI_EVENT_TRIGGERS]; /* 0x0C */
+} MPI26_CONFIG_PAGE_DRIVER_TRIGGER_2, Mpi26DriverTriggerPage2_t;
#define MPI26_DRIVER_TRIGGER_PAGE3_PAGEVERSION (0x01)
-typedef struct _MPI26_DRIVER_SCSI_SENSE_TIGGER_ENTRY {
+typedef struct _MPI26_DRIVER_SCSI_SENSE_TRIGGER_ENTRY {
U8 ASCQ; /* 0x00 */
U8 ASC; /* 0x01 */
U8 SenseKey; /* 0x02 */
U8 Reserved; /* 0x03 */
-} MPI26_DRIVER_SCSI_SENSE_TIGGER_ENTRY;
+} MPI26_DRIVER_SCSI_SENSE_TRIGGER_ENTRY;
#define MPI26_MAX_SCSI_SENSE_TRIGGERS (20)
-typedef struct _MPI26_CONFIG_PAGE_DRIVER_TIGGER_3 {
+typedef struct _MPI26_CONFIG_PAGE_DRIVER_TRIGGER_3 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /* 0x00 */
U16 NumSCSISenseTrigger; /* 0x08 */
U16 Reserved0xA; /* 0x0A */
- MPI26_DRIVER_SCSI_SENSE_TIGGER_ENTRY SCSISenseTriggers[MPI26_MAX_SCSI_SENSE_TRIGGERS]; /* 0x0C */
-} MPI26_CONFIG_PAGE_DRIVER_TIGGER_3, Mpi26DriverTriggerPage3_t;
+ MPI26_DRIVER_SCSI_SENSE_TRIGGER_ENTRY SCSISenseTriggers[MPI26_MAX_SCSI_SENSE_TRIGGERS]; /* 0x0C */
+} MPI26_CONFIG_PAGE_DRIVER_TRIGGER_3, Mpi26DriverTriggerPage3_t;
#define MPI26_DRIVER_TRIGGER_PAGE4_PAGEVERSION (0x01)
-typedef struct _MPI26_DRIVER_IOCSTATUS_LOGINFO_TIGGER_ENTRY {
+typedef struct _MPI26_DRIVER_IOCSTATUS_LOGINFO_TRIGGER_ENTRY {
U16 IOCStatus; /* 0x00 */
U16 Reserved; /* 0x02 */
U32 LogInfo; /* 0x04 */
-} MPI26_DRIVER_IOCSTATUS_LOGINFO_TIGGER_ENTRY;
+} MPI26_DRIVER_IOCSTATUS_LOGINFO_TRIGGER_ENTRY;
#define MPI26_MAX_LOGINFO_TRIGGERS (20)
-typedef struct _MPI26_CONFIG_PAGE_DRIVER_TIGGER_4 {
+typedef struct _MPI26_CONFIG_PAGE_DRIVER_TRIGGER_4 {
MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /* 0x00 */
U16 NumIOCStatusLogInfoTrigger; /* 0x08 */
U16 Reserved0xA; /* 0x0A */
- MPI26_DRIVER_IOCSTATUS_LOGINFO_TIGGER_ENTRY IOCStatusLoginfoTriggers[MPI26_MAX_LOGINFO_TRIGGERS]; /* 0x0C */
-} MPI26_CONFIG_PAGE_DRIVER_TIGGER_4, Mpi26DriverTriggerPage4_t;
+ MPI26_DRIVER_IOCSTATUS_LOGINFO_TRIGGER_ENTRY IOCStatusLoginfoTriggers[MPI26_MAX_LOGINFO_TRIGGERS]; /* 0x0C */
+} MPI26_CONFIG_PAGE_DRIVER_TRIGGER_4, Mpi26DriverTriggerPage4_t;
#endif
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 6/6] scsi: mpt3sas: Replace a dynamic allocation with a local variable
2023-07-25 16:13 [PATCH 0/6] scsi: mpt3sas: Use flexible arrays and do a few cleanups James Seo
` (4 preceding siblings ...)
2023-07-25 16:13 ` [PATCH 5/6] scsi: mpt3sas: Fix typo of "TRIGGER" James Seo
@ 2023-07-25 16:13 ` James Seo
2023-07-28 22:29 ` Kees Cook
5 siblings, 1 reply; 14+ messages in thread
From: James Seo @ 2023-07-25 16:13 UTC (permalink / raw)
To: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani
Cc: James Seo, James E.J. Bottomley, Martin K. Petersen, Kees Cook,
Gustavo A. R. Silva, MPT-FusionLinux.pdl, linux-scsi,
linux-kernel
This dynamic allocation can be replaced with a local variable.
Signed-off-by: James Seo <james@equiv.tech>
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index cd6f36094159..a32a6fa728a7 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -5361,10 +5361,9 @@ _base_update_diag_trigger_pages(struct MPT3SAS_ADAPTER *ioc)
static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc)
{
Mpi2ConfigReply_t mpi_reply;
- Mpi2SasIOUnitPage1_t *sas_iounit_pg1 = NULL;
+ Mpi2SasIOUnitPage1_t sas_iounit_pg1;
Mpi26PCIeIOUnitPage1_t pcie_iounit_pg1;
u16 depth;
- int sz;
int rc = 0;
ioc->max_wideport_qd = MPT3SAS_SAS_QUEUE_DEPTH;
@@ -5374,28 +5373,21 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc)
if (!ioc->is_gen35_ioc)
goto out;
/* sas iounit page 1 */
- sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData);
- sas_iounit_pg1 = kzalloc(sizeof(Mpi2SasIOUnitPage1_t), GFP_KERNEL);
- if (!sas_iounit_pg1) {
- pr_err("%s: failure at %s:%d/%s()!\n",
- ioc->name, __FILE__, __LINE__, __func__);
- return rc;
- }
rc = mpt3sas_config_get_sas_iounit_pg1(ioc, &mpi_reply,
- sas_iounit_pg1, sz);
+ &sas_iounit_pg1, sizeof(Mpi2SasIOUnitPage1_t));
if (rc) {
pr_err("%s: failure at %s:%d/%s()!\n",
ioc->name, __FILE__, __LINE__, __func__);
goto out;
}
- depth = le16_to_cpu(sas_iounit_pg1->SASWideMaxQueueDepth);
+ depth = le16_to_cpu(sas_iounit_pg1.SASWideMaxQueueDepth);
ioc->max_wideport_qd = (depth ? depth : MPT3SAS_SAS_QUEUE_DEPTH);
- depth = le16_to_cpu(sas_iounit_pg1->SASNarrowMaxQueueDepth);
+ depth = le16_to_cpu(sas_iounit_pg1.SASNarrowMaxQueueDepth);
ioc->max_narrowport_qd = (depth ? depth : MPT3SAS_SAS_QUEUE_DEPTH);
- depth = sas_iounit_pg1->SATAMaxQDepth;
+ depth = sas_iounit_pg1.SATAMaxQDepth;
ioc->max_sata_qd = (depth ? depth : MPT3SAS_SATA_QUEUE_DEPTH);
/* pcie iounit page 1 */
@@ -5414,7 +5406,6 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc)
"MaxWidePortQD: 0x%x MaxNarrowPortQD: 0x%x MaxSataQD: 0x%x MaxNvmeQD: 0x%x\n",
ioc->max_wideport_qd, ioc->max_narrowport_qd,
ioc->max_sata_qd, ioc->max_nvme_qd));
- kfree(sas_iounit_pg1);
return rc;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] scsi: mpt3sas: Use flexible arrays when obviously possible
2023-07-25 16:13 ` [PATCH 1/6] scsi: mpt3sas: Use flexible arrays when obviously possible James Seo
@ 2023-07-28 22:01 ` Kees Cook
0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2023-07-28 22:01 UTC (permalink / raw)
To: James Seo
Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani,
James E.J. Bottomley, Martin K. Petersen, Gustavo A. R. Silva,
MPT-FusionLinux.pdl, linux-scsi, linux-kernel
On Tue, Jul 25, 2023 at 09:13:26AM -0700, James Seo wrote:
> These old-style 1-length variable arrays can be directly converted
> into C99 flexible array members without any binary changes.
>
> In most cases, they belong to unused structs, or to structs used only
> by unused code. The remaining few coincidentally have their sizes
> calculated in roundabout ways that do not depend on the sizeof()
> their structs.
>
> Signed-off-by: James Seo <james@equiv.tech>
Thanks for tackling these fixes!
Doing before/after build testing[1] shows no binary differences with that patch.
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
[1] https://outflux.net/blog/archives/2022/06/24/finding-binary-differences/
> ---
> drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 181 ++++++++------------------
> drivers/scsi/mpt3sas/mpi/mpi2_image.h | 32 ++---
> drivers/scsi/mpt3sas/mpi/mpi2_ioc.h | 27 ++--
> 3 files changed, 75 insertions(+), 165 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
> index 4d0be5ab98c1..42d820159c44 100644
> --- a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
> +++ b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
> @@ -643,18 +643,14 @@ typedef struct _MPI2_CHIP_REVISION_ID {
> /*Manufacturing Page 2 */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check Header.PageLength at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check Header.PageLength at
> + *runtime before using HwSettings[].
> */
> -#ifndef MPI2_MAN_PAGE_2_HW_SETTINGS_WORDS
> -#define MPI2_MAN_PAGE_2_HW_SETTINGS_WORDS (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_MAN_2 {
> MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
> MPI2_CHIP_REVISION_ID ChipId; /*0x04 */
> - U32
> - HwSettings[MPI2_MAN_PAGE_2_HW_SETTINGS_WORDS];/*0x08 */
> + U32 HwSettings[]; /*0x08 */
> } MPI2_CONFIG_PAGE_MAN_2,
> *PTR_MPI2_CONFIG_PAGE_MAN_2,
> Mpi2ManufacturingPage2_t,
> @@ -666,18 +662,14 @@ typedef struct _MPI2_CONFIG_PAGE_MAN_2 {
> /*Manufacturing Page 3 */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check Header.PageLength at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check Header.PageLength at
> + *runtime before using Info[].
> */
> -#ifndef MPI2_MAN_PAGE_3_INFO_WORDS
> -#define MPI2_MAN_PAGE_3_INFO_WORDS (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_MAN_3 {
> MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
> MPI2_CHIP_REVISION_ID ChipId; /*0x04 */
> - U32
> - Info[MPI2_MAN_PAGE_3_INFO_WORDS];/*0x08 */
> + U32 Info[]; /*0x08 */
> } MPI2_CONFIG_PAGE_MAN_3,
> *PTR_MPI2_CONFIG_PAGE_MAN_3,
> Mpi2ManufacturingPage3_t,
> @@ -765,12 +757,9 @@ typedef struct _MPI2_CONFIG_PAGE_MAN_4 {
> /*Manufacturing Page 5 */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumPhys at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumPhys at runtime before using Phy[].
> */
> -#ifndef MPI2_MAN_PAGE_5_PHY_ENTRIES
> -#define MPI2_MAN_PAGE_5_PHY_ENTRIES (1)
> -#endif
>
> typedef struct _MPI2_MANUFACTURING5_ENTRY {
> U64 WWID; /*0x00 */
> @@ -787,8 +776,7 @@ typedef struct _MPI2_CONFIG_PAGE_MAN_5 {
> U16 Reserved2; /*0x06 */
> U32 Reserved3; /*0x08 */
> U32 Reserved4; /*0x0C */
> - MPI2_MANUFACTURING5_ENTRY
> - Phy[MPI2_MAN_PAGE_5_PHY_ENTRIES];/*0x08 */
> + MPI2_MANUFACTURING5_ENTRY Phy[]; /*0x10 */
> } MPI2_CONFIG_PAGE_MAN_5,
> *PTR_MPI2_CONFIG_PAGE_MAN_5,
> Mpi2ManufacturingPage5_t,
> @@ -864,12 +852,9 @@ typedef struct _MPI2_MANPAGE7_CONNECTOR_INFO {
> #define MPI2_MANPAGE7_SLOT_UNKNOWN (0xFFFF)
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumPhys at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumPhys at runtime before using ConnectorInfo[].
> */
> -#ifndef MPI2_MANPAGE7_CONNECTOR_INFO_MAX
> -#define MPI2_MANPAGE7_CONNECTOR_INFO_MAX (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_MAN_7 {
> MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
> @@ -880,8 +865,7 @@ typedef struct _MPI2_CONFIG_PAGE_MAN_7 {
> U8 NumPhys; /*0x20 */
> U8 Reserved3; /*0x21 */
> U16 Reserved4; /*0x22 */
> - MPI2_MANPAGE7_CONNECTOR_INFO
> - ConnectorInfo[MPI2_MANPAGE7_CONNECTOR_INFO_MAX]; /*0x24 */
> + MPI2_MANPAGE7_CONNECTOR_INFO ConnectorInfo[]; /*0x24 */
> } MPI2_CONFIG_PAGE_MAN_7,
> *PTR_MPI2_CONFIG_PAGE_MAN_7,
> Mpi2ManufacturingPage7_t,
> @@ -1019,12 +1003,9 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_3 {
> /*IO Unit Page 5 */
>
> /*
> - *Upper layer code (drivers, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumDmaEngines at runtime.
> + *Upper layer code (drivers, utilities, etc.) should check the value returned
> + *for NumDmaEngines at runtime before using DmaEngineCapabilities[].
> */
> -#ifndef MPI2_IOUNITPAGE5_DMAENGINE_ENTRIES
> -#define MPI2_IOUNITPAGE5_DMAENGINE_ENTRIES (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_5 {
> MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
> @@ -1042,7 +1023,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_5 {
> U32 Reserved2; /*0x24 */
> U32 Reserved3; /*0x28 */
> U32
> - DmaEngineCapabilities[MPI2_IOUNITPAGE5_DMAENGINE_ENTRIES]; /*0x2C */
> + DmaEngineCapabilities[]; /*0x2C */
> } MPI2_CONFIG_PAGE_IO_UNIT_5,
> *PTR_MPI2_CONFIG_PAGE_IO_UNIT_5,
> Mpi2IOUnitPage5_t, *pMpi2IOUnitPage5_t;
> @@ -1259,12 +1240,9 @@ typedef struct _MPI2_IOUNIT9_SENSOR {
> #define MPI2_IOUNIT9_SENSOR_FLAGS_TEMP_VALID (0x01)
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumSensors at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumSensors at runtime before using Sensor[].
> */
> -#ifndef MPI2_IOUNITPAGE9_SENSOR_ENTRIES
> -#define MPI2_IOUNITPAGE9_SENSOR_ENTRIES (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_9 {
> MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
> @@ -1273,8 +1251,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_9 {
> U8 NumSensors; /*0x0C */
> U8 Reserved4; /*0x0D */
> U16 Reserved3; /*0x0E */
> - MPI2_IOUNIT9_SENSOR
> - Sensor[MPI2_IOUNITPAGE9_SENSOR_ENTRIES];/*0x10 */
> + MPI2_IOUNIT9_SENSOR Sensor[]; /*0x10 */
> } MPI2_CONFIG_PAGE_IO_UNIT_9,
> *PTR_MPI2_CONFIG_PAGE_IO_UNIT_9,
> Mpi2IOUnitPage9_t, *pMpi2IOUnitPage9_t;
> @@ -1294,12 +1271,9 @@ typedef struct _MPI2_IOUNIT10_FUNCTION {
> *pMpi2IOUnit10Function_t;
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumFunctions at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumFunctions at runtime before using Function[].
> */
> -#ifndef MPI2_IOUNITPAGE10_FUNCTION_ENTRIES
> -#define MPI2_IOUNITPAGE10_FUNCTION_ENTRIES (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_10 {
> MPI2_CONFIG_PAGE_HEADER Header; /*0x00 */
> @@ -1308,8 +1282,7 @@ typedef struct _MPI2_CONFIG_PAGE_IO_UNIT_10 {
> U16 Reserved2; /*0x06 */
> U32 Reserved3; /*0x08 */
> U32 Reserved4; /*0x0C */
> - MPI2_IOUNIT10_FUNCTION
> - Function[MPI2_IOUNITPAGE10_FUNCTION_ENTRIES];/*0x10 */
> + MPI2_IOUNIT10_FUNCTION Function[]; /*0x10 */
> } MPI2_CONFIG_PAGE_IO_UNIT_10,
> *PTR_MPI2_CONFIG_PAGE_IO_UNIT_10,
> Mpi2IOUnitPage10_t, *pMpi2IOUnitPage10_t;
> @@ -1764,12 +1737,9 @@ typedef struct _MPI2_CONFIG_PAGE_BIOS_3 {
> /*BIOS Page 4 */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumPhys at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumPhys at runtime before using Phy[].
> */
> -#ifndef MPI2_BIOS_PAGE_4_PHY_ENTRIES
> -#define MPI2_BIOS_PAGE_4_PHY_ENTRIES (1)
> -#endif
>
> typedef struct _MPI2_BIOS4_ENTRY {
> U64 ReassignmentWWID; /*0x00 */
> @@ -1782,8 +1752,7 @@ typedef struct _MPI2_CONFIG_PAGE_BIOS_4 {
> U8 NumPhys; /*0x04 */
> U8 Reserved1; /*0x05 */
> U16 Reserved2; /*0x06 */
> - MPI2_BIOS4_ENTRY
> - Phy[MPI2_BIOS_PAGE_4_PHY_ENTRIES]; /*0x08 */
> + MPI2_BIOS4_ENTRY Phy[]; /*0x08 */
> } MPI2_CONFIG_PAGE_BIOS_4, *PTR_MPI2_CONFIG_PAGE_BIOS_4,
> Mpi2BiosPage4_t, *pMpi2BiosPage4_t;
>
> @@ -2045,12 +2014,9 @@ typedef struct _MPI2_CONFIG_PAGE_RD_PDISK_0 {
> /*RAID Physical Disk Page 1 */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumPhysDiskPaths at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumPhysDiskPaths at runtime before using PhysicalDiskPath[].
> */
> -#ifndef MPI2_RAID_PHYS_DISK1_PATH_MAX
> -#define MPI2_RAID_PHYS_DISK1_PATH_MAX (1)
> -#endif
>
> typedef struct _MPI2_RAIDPHYSDISK1_PATH {
> U16 DevHandle; /*0x00 */
> @@ -2075,8 +2041,7 @@ typedef struct _MPI2_CONFIG_PAGE_RD_PDISK_1 {
> U8 PhysDiskNum; /*0x05 */
> U16 Reserved1; /*0x06 */
> U32 Reserved2; /*0x08 */
> - MPI2_RAIDPHYSDISK1_PATH
> - PhysicalDiskPath[MPI2_RAID_PHYS_DISK1_PATH_MAX];/*0x0C */
> + MPI2_RAIDPHYSDISK1_PATH PhysicalDiskPath[]; /*0x0C */
> } MPI2_CONFIG_PAGE_RD_PDISK_1,
> *PTR_MPI2_CONFIG_PAGE_RD_PDISK_1,
> Mpi2RaidPhysDiskPage1_t,
> @@ -2502,12 +2467,9 @@ typedef struct _MPI2_SAS_IO_UNIT5_PHY_PM_SETTINGS {
> #define MPI2_SASIOUNIT5_ITE_ONE_MICROSECOND (0)
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumPhys at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumPhys at runtime before using SASPhyPowerManagementSettings[].
> */
> -#ifndef MPI2_SAS_IOUNIT5_PHY_MAX
> -#define MPI2_SAS_IOUNIT5_PHY_MAX (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_5 {
> MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
> @@ -2516,7 +2478,7 @@ typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_5 {
> U16 Reserved2;/*0x0A */
> U32 Reserved3;/*0x0C */
> MPI2_SAS_IO_UNIT5_PHY_PM_SETTINGS
> - SASPhyPowerManagementSettings[MPI2_SAS_IOUNIT5_PHY_MAX];/*0x10 */
> + SASPhyPowerManagementSettings[]; /*0x10 */
> } MPI2_CONFIG_PAGE_SASIOUNIT_5,
> *PTR_MPI2_CONFIG_PAGE_SASIOUNIT_5,
> Mpi2SasIOUnitPage5_t, *pMpi2SasIOUnitPage5_t;
> @@ -2554,12 +2516,9 @@ typedef struct _MPI2_SAS_IO_UNIT6_PORT_WIDTH_MOD_GROUP_STATUS {
> #define MPI2_SASIOUNIT6_MODULATION_100_PERCENT (0x03)
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumGroups at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumGroups at runtime before using PortWidthModulationGroupStatus[].
> */
> -#ifndef MPI2_SAS_IOUNIT6_GROUP_MAX
> -#define MPI2_SAS_IOUNIT6_GROUP_MAX (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_6 {
> MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
> @@ -2569,7 +2528,7 @@ typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_6 {
> U8 Reserved3; /*0x11 */
> U16 Reserved4; /*0x12 */
> MPI2_SAS_IO_UNIT6_PORT_WIDTH_MOD_GROUP_STATUS
> - PortWidthModulationGroupStatus[MPI2_SAS_IOUNIT6_GROUP_MAX]; /*0x14 */
> + PortWidthModulationGroupStatus[]; /*0x14 */
> } MPI2_CONFIG_PAGE_SASIOUNIT_6,
> *PTR_MPI2_CONFIG_PAGE_SASIOUNIT_6,
> Mpi2SasIOUnitPage6_t, *pMpi2SasIOUnitPage6_t;
> @@ -2597,12 +2556,9 @@ typedef struct _MPI2_SAS_IO_UNIT7_PORT_WIDTH_MOD_GROUP_SETTINGS {
>
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumGroups at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumGroups at runtime before using PortWidthModulationGroupSettings[].
> */
> -#ifndef MPI2_SAS_IOUNIT7_GROUP_MAX
> -#define MPI2_SAS_IOUNIT7_GROUP_MAX (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_7 {
> MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
> @@ -2615,7 +2571,7 @@ typedef struct _MPI2_CONFIG_PAGE_SASIOUNIT_7 {
> U8 Reserved4; /*0x15 */
> U16 Reserved5; /*0x16 */
> MPI2_SAS_IO_UNIT7_PORT_WIDTH_MOD_GROUP_SETTINGS
> - PortWidthModulationGroupSettings[MPI2_SAS_IOUNIT7_GROUP_MAX];/*0x18 */
> + PortWidthModulationGroupSettings[]; /*0x18 */
> } MPI2_CONFIG_PAGE_SASIOUNIT_7,
> *PTR_MPI2_CONFIG_PAGE_SASIOUNIT_7,
> Mpi2SasIOUnitPage7_t, *pMpi2SasIOUnitPage7_t;
> @@ -3086,12 +3042,9 @@ typedef struct _MPI2_SASPHY2_PHY_EVENT {
>
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumPhyEvents at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumPhyEvents at runtime before using PhyEvent[].
> */
> -#ifndef MPI2_SASPHY2_PHY_EVENT_MAX
> -#define MPI2_SASPHY2_PHY_EVENT_MAX (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_SAS_PHY_2 {
> MPI2_CONFIG_EXTENDED_PAGE_HEADER
> @@ -3105,7 +3058,7 @@ typedef struct _MPI2_CONFIG_PAGE_SAS_PHY_2 {
> U16
> Reserved3; /*0x0E */
> MPI2_SASPHY2_PHY_EVENT
> - PhyEvent[MPI2_SASPHY2_PHY_EVENT_MAX]; /*0x10 */
> + PhyEvent[]; /*0x10 */
> } MPI2_CONFIG_PAGE_SAS_PHY_2,
> *PTR_MPI2_CONFIG_PAGE_SAS_PHY_2,
> Mpi2SasPhyPage2_t,
> @@ -3200,12 +3153,9 @@ typedef struct _MPI2_SASPHY3_PHY_EVENT_CONFIG {
> #define MPI2_SASPHY3_TFLAGS_EVENT_NOTIFY (0x0001)
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumPhyEvents at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumPhyEvents at runtime before using PhyEventConfig[].
> */
> -#ifndef MPI2_SASPHY3_PHY_EVENT_MAX
> -#define MPI2_SASPHY3_PHY_EVENT_MAX (1)
> -#endif
>
> typedef struct _MPI2_CONFIG_PAGE_SAS_PHY_3 {
> MPI2_CONFIG_EXTENDED_PAGE_HEADER
> @@ -3219,7 +3169,7 @@ typedef struct _MPI2_CONFIG_PAGE_SAS_PHY_3 {
> U16
> Reserved3; /*0x0E */
> MPI2_SASPHY3_PHY_EVENT_CONFIG
> - PhyEventConfig[MPI2_SASPHY3_PHY_EVENT_MAX]; /*0x10 */
> + PhyEventConfig[]; /*0x10 */
> } MPI2_CONFIG_PAGE_SAS_PHY_3,
> *PTR_MPI2_CONFIG_PAGE_SAS_PHY_3,
> Mpi2SasPhyPage3_t, *pMpi2SasPhyPage3_t;
> @@ -3358,12 +3308,9 @@ typedef struct _MPI2_CONFIG_PAGE_SAS_ENCLOSURE_0 {
> /*Log Page 0 */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumLogEntries at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumLogEntries at runtime before using LogEntry[].
> */
> -#ifndef MPI2_LOG_0_NUM_LOG_ENTRIES
> -#define MPI2_LOG_0_NUM_LOG_ENTRIES (1)
> -#endif
>
> #define MPI2_LOG_0_LOG_DATA_LENGTH (0x1C)
>
> @@ -3393,8 +3340,7 @@ typedef struct _MPI2_CONFIG_PAGE_LOG_0 {
> U32 Reserved2; /*0x0C */
> U16 NumLogEntries;/*0x10 */
> U16 Reserved3; /*0x12 */
> - MPI2_LOG_0_ENTRY
> - LogEntry[MPI2_LOG_0_NUM_LOG_ENTRIES]; /*0x14 */
> + MPI2_LOG_0_ENTRY LogEntry[]; /*0x14 */
> } MPI2_CONFIG_PAGE_LOG_0, *PTR_MPI2_CONFIG_PAGE_LOG_0,
> Mpi2LogPage0_t, *pMpi2LogPage0_t;
>
> @@ -3408,12 +3354,9 @@ typedef struct _MPI2_CONFIG_PAGE_LOG_0 {
> /*RAID Page 0 */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumElements at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumElements at runtime before using ConfigElement[].
> */
> -#ifndef MPI2_RAIDCONFIG0_MAX_ELEMENTS
> -#define MPI2_RAIDCONFIG0_MAX_ELEMENTS (1)
> -#endif
>
> typedef struct _MPI2_RAIDCONFIG0_CONFIG_ELEMENT {
> U16 ElementFlags; /*0x00 */
> @@ -3446,8 +3389,7 @@ typedef struct _MPI2_CONFIG_PAGE_RAID_CONFIGURATION_0 {
> U8 NumElements; /*0x2C */
> U8 Reserved2; /*0x2D */
> U16 Reserved3; /*0x2E */
> - MPI2_RAIDCONFIG0_CONFIG_ELEMENT
> - ConfigElement[MPI2_RAIDCONFIG0_MAX_ELEMENTS]; /*0x30 */
> + MPI2_RAIDCONFIG0_CONFIG_ELEMENT ConfigElement[];/*0x30 */
> } MPI2_CONFIG_PAGE_RAID_CONFIGURATION_0,
> *PTR_MPI2_CONFIG_PAGE_RAID_CONFIGURATION_0,
> Mpi2RaidConfigurationPage0_t,
> @@ -3687,12 +3629,9 @@ typedef struct _MPI26_PCIE_IO_UNIT0_PHY_DATA {
> Mpi26PCIeIOUnit0PhyData_t, *pMpi26PCIeIOUnit0PhyData_t;
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumPhys at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumPhys at runtime before using PhyData[].
> */
> -#ifndef MPI26_PCIE_IOUNIT0_PHY_MAX
> -#define MPI26_PCIE_IOUNIT0_PHY_MAX (1)
> -#endif
>
> typedef struct _MPI26_CONFIG_PAGE_PIOUNIT_0 {
> MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
> @@ -3701,7 +3640,7 @@ typedef struct _MPI26_CONFIG_PAGE_PIOUNIT_0 {
> U8 InitStatus; /*0x0D */
> U16 Reserved3; /*0x0E */
> MPI26_PCIE_IO_UNIT0_PHY_DATA
> - PhyData[MPI26_PCIE_IOUNIT0_PHY_MAX]; /*0x10 */
> + PhyData[]; /*0x10 */
> } MPI26_CONFIG_PAGE_PIOUNIT_0,
> *PTR_MPI26_CONFIG_PAGE_PIOUNIT_0,
> Mpi26PCIeIOUnitPage0_t, *pMpi26PCIeIOUnitPage0_t;
> @@ -3993,12 +3932,9 @@ typedef struct _MPI26_PCIELINK2_LINK_EVENT {
>
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumLinkEvents at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumLinkEvents at runtime before using LinkEvent[].
> */
> -#ifndef MPI26_PCIELINK2_LINK_EVENT_MAX
> -#define MPI26_PCIELINK2_LINK_EVENT_MAX (1)
> -#endif
>
> typedef struct _MPI26_CONFIG_PAGE_PCIELINK_2 {
> MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
> @@ -4009,7 +3945,7 @@ typedef struct _MPI26_CONFIG_PAGE_PCIELINK_2 {
> U8 Reserved3; /*0x0D */
> U16 Reserved4; /*0x0E */
> MPI26_PCIELINK2_LINK_EVENT
> - LinkEvent[MPI26_PCIELINK2_LINK_EVENT_MAX]; /*0x10 */
> + LinkEvent[]; /*0x10 */
> } MPI26_CONFIG_PAGE_PCIELINK_2, *PTR_MPI26_CONFIG_PAGE_PCIELINK_2,
> Mpi26PcieLinkPage2_t, *pMpi26PcieLinkPage2_t;
>
> @@ -4067,12 +4003,9 @@ typedef struct _MPI26_PCIELINK3_LINK_EVENT_CONFIG {
> #define MPI26_PCIELINK3_TFLAGS_EVENT_NOTIFY (0x0001)
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check the value returned for NumLinkEvents at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check the value returned
> + *for NumLinkEvents at runtime before using LinkEventConfig[].
> */
> -#ifndef MPI26_PCIELINK3_LINK_EVENT_MAX
> -#define MPI26_PCIELINK3_LINK_EVENT_MAX (1)
> -#endif
>
> typedef struct _MPI26_CONFIG_PAGE_PCIELINK_3 {
> MPI2_CONFIG_EXTENDED_PAGE_HEADER Header; /*0x00 */
> @@ -4083,7 +4016,7 @@ typedef struct _MPI26_CONFIG_PAGE_PCIELINK_3 {
> U8 Reserved3; /*0x0D */
> U16 Reserved4; /*0x0E */
> MPI26_PCIELINK3_LINK_EVENT_CONFIG
> - LinkEventConfig[MPI26_PCIELINK3_LINK_EVENT_MAX]; /*0x10 */
> + LinkEventConfig[]; /*0x10 */
> } MPI26_CONFIG_PAGE_PCIELINK_3, *PTR_MPI26_CONFIG_PAGE_PCIELINK_3,
> Mpi26PcieLinkPage3_t, *pMpi26PcieLinkPage3_t;
>
> diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_image.h b/drivers/scsi/mpt3sas/mpi/mpi2_image.h
> index 33b9c3a6fd40..798ab6e33eb9 100644
> --- a/drivers/scsi/mpt3sas/mpi/mpi2_image.h
> +++ b/drivers/scsi/mpt3sas/mpi/mpi2_image.h
> @@ -295,20 +295,9 @@ typedef struct _MPI2_EXT_IMAGE_HEADER {
> /*FLASH Layout Extended Image Data */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check RegionsPerLayout at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check NumberOfLayouts and
> + *RegionsPerLayout at runtime before using Layout[] and Region[].
> */
> -#ifndef MPI2_FLASH_NUMBER_OF_REGIONS
> -#define MPI2_FLASH_NUMBER_OF_REGIONS (1)
> -#endif
> -
> -/*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check NumberOfLayouts at runtime.
> - */
> -#ifndef MPI2_FLASH_NUMBER_OF_LAYOUTS
> -#define MPI2_FLASH_NUMBER_OF_LAYOUTS (1)
> -#endif
>
> typedef struct _MPI2_FLASH_REGION {
> U8 RegionType; /*0x00 */
> @@ -325,7 +314,7 @@ typedef struct _MPI2_FLASH_LAYOUT {
> U32 Reserved1; /*0x04 */
> U32 Reserved2; /*0x08 */
> U32 Reserved3; /*0x0C */
> - MPI2_FLASH_REGION Region[MPI2_FLASH_NUMBER_OF_REGIONS]; /*0x10 */
> + MPI2_FLASH_REGION Region[]; /*0x10 */
> } MPI2_FLASH_LAYOUT, *PTR_MPI2_FLASH_LAYOUT,
> Mpi2FlashLayout_t, *pMpi2FlashLayout_t;
>
> @@ -339,7 +328,7 @@ typedef struct _MPI2_FLASH_LAYOUT_DATA {
> U16 MinimumSectorAlignment; /*0x08 */
> U16 Reserved3; /*0x0A */
> U32 Reserved4; /*0x0C */
> - MPI2_FLASH_LAYOUT Layout[MPI2_FLASH_NUMBER_OF_LAYOUTS]; /*0x10 */
> + MPI2_FLASH_LAYOUT Layout[]; /*0x10 */
> } MPI2_FLASH_LAYOUT_DATA, *PTR_MPI2_FLASH_LAYOUT_DATA,
> Mpi2FlashLayoutData_t, *pMpi2FlashLayoutData_t;
>
> @@ -373,12 +362,9 @@ typedef struct _MPI2_FLASH_LAYOUT_DATA {
> /*Supported Devices Extended Image Data */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check NumberOfDevices at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check NumberOfDevices at
> + *runtime before using SupportedDevice[].
> */
> -#ifndef MPI2_SUPPORTED_DEVICES_IMAGE_NUM_DEVICES
> -#define MPI2_SUPPORTED_DEVICES_IMAGE_NUM_DEVICES (1)
> -#endif
>
> typedef struct _MPI2_SUPPORTED_DEVICE {
> U16 DeviceID; /*0x00 */
> @@ -399,7 +385,7 @@ typedef struct _MPI2_SUPPORTED_DEVICES_DATA {
> U8 Reserved2; /*0x03 */
> U32 Reserved3; /*0x04 */
> MPI2_SUPPORTED_DEVICE
> - SupportedDevice[MPI2_SUPPORTED_DEVICES_IMAGE_NUM_DEVICES];/*0x08 */
> + SupportedDevice[]; /*0x08 */
> } MPI2_SUPPORTED_DEVICES_DATA, *PTR_MPI2_SUPPORTED_DEVICES_DATA,
> Mpi2SupportedDevicesData_t, *pMpi2SupportedDevicesData_t;
>
> @@ -464,7 +450,7 @@ typedef struct _MPI25_ENCRYPTED_HASH_ENTRY {
> U8 EncryptionAlgorithm; /*0x02 */
> U8 Reserved1; /*0x03 */
> U32 Reserved2; /*0x04 */
> - U32 EncryptedHash[1]; /*0x08 */ /* variable length */
> + U32 EncryptedHash[]; /*0x08 */
> } MPI25_ENCRYPTED_HASH_ENTRY, *PTR_MPI25_ENCRYPTED_HASH_ENTRY,
> Mpi25EncryptedHashEntry_t, *pMpi25EncryptedHashEntry_t;
>
> @@ -508,7 +494,7 @@ typedef struct _MPI25_ENCRYPTED_HASH_DATA {
> U8 NumHash; /*0x01 */
> U16 Reserved1; /*0x02 */
> U32 Reserved2; /*0x04 */
> - MPI25_ENCRYPTED_HASH_ENTRY EncryptedHashEntry[1]; /*0x08 */
> + MPI25_ENCRYPTED_HASH_ENTRY EncryptedHashEntry[]; /*0x08 */
> } MPI25_ENCRYPTED_HASH_DATA, *PTR_MPI25_ENCRYPTED_HASH_DATA,
> Mpi25EncryptedHashData_t, *pMpi25EncryptedHashData_t;
>
> diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h b/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
> index 2c57115172cf..d92852591134 100644
> --- a/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
> +++ b/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
> @@ -808,12 +808,9 @@ typedef struct _MPI2_EVENT_DATA_IR_PHYSICAL_DISK {
> /*Integrated RAID Configuration Change List Event data */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check NumElements at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check NumElements at
> + *runtime before using ConfigElement[].
> */
> -#ifndef MPI2_EVENT_IR_CONFIG_ELEMENT_COUNT
> -#define MPI2_EVENT_IR_CONFIG_ELEMENT_COUNT (1)
> -#endif
>
> typedef struct _MPI2_EVENT_IR_CONFIG_ELEMENT {
> U16 ElementFlags; /*0x00 */
> @@ -848,7 +845,7 @@ typedef struct _MPI2_EVENT_DATA_IR_CONFIG_CHANGE_LIST {
> U8 ConfigNum; /*0x03 */
> U32 Flags; /*0x04 */
> MPI2_EVENT_IR_CONFIG_ELEMENT
> - ConfigElement[MPI2_EVENT_IR_CONFIG_ELEMENT_COUNT];/*0x08 */
> + ConfigElement[];/*0x08 */
> } MPI2_EVENT_DATA_IR_CONFIG_CHANGE_LIST,
> *PTR_MPI2_EVENT_DATA_IR_CONFIG_CHANGE_LIST,
> Mpi2EventDataIrConfigChangeList_t,
> @@ -969,12 +966,9 @@ typedef struct _MPI2_EVENT_DATA_SAS_INIT_TABLE_OVERFLOW {
> /*SAS Topology Change List Event data */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check NumEntries at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check NumEntries at
> + *runtime before using PHY[].
> */
> -#ifndef MPI2_EVENT_SAS_TOPO_PHY_COUNT
> -#define MPI2_EVENT_SAS_TOPO_PHY_COUNT (1)
> -#endif
>
> typedef struct _MPI2_EVENT_SAS_TOPO_PHY_ENTRY {
> U16 AttachedDevHandle; /*0x00 */
> @@ -994,7 +988,7 @@ typedef struct _MPI2_EVENT_DATA_SAS_TOPOLOGY_CHANGE_LIST {
> U8 ExpStatus; /*0x0A */
> U8 PhysicalPort; /*0x0B */
> MPI2_EVENT_SAS_TOPO_PHY_ENTRY
> - PHY[MPI2_EVENT_SAS_TOPO_PHY_COUNT]; /*0x0C */
> + PHY[]; /*0x0C */
> } MPI2_EVENT_DATA_SAS_TOPOLOGY_CHANGE_LIST,
> *PTR_MPI2_EVENT_DATA_SAS_TOPOLOGY_CHANGE_LIST,
> Mpi2EventDataSasTopologyChangeList_t,
> @@ -1229,12 +1223,9 @@ typedef struct _MPI26_EVENT_DATA_PCIE_ENUMERATION {
> /*PCIe Topology Change List Event data (MPI v2.6 and later) */
>
> /*
> - *Host code (drivers, BIOS, utilities, etc.) should leave this define set to
> - *one and check NumEntries at runtime.
> + *Host code (drivers, BIOS, utilities, etc.) should check NumEntries at
> + *runtime before using PortEntry[].
> */
> -#ifndef MPI26_EVENT_PCIE_TOPO_PORT_COUNT
> -#define MPI26_EVENT_PCIE_TOPO_PORT_COUNT (1)
> -#endif
>
> typedef struct _MPI26_EVENT_PCIE_TOPO_PORT_ENTRY {
> U16 AttachedDevHandle; /*0x00 */
> @@ -1286,7 +1277,7 @@ typedef struct _MPI26_EVENT_DATA_PCIE_TOPOLOGY_CHANGE_LIST {
> U8 SwitchStatus; /*0x0A */
> U8 PhysicalPort; /*0x0B */
> MPI26_EVENT_PCIE_TOPO_PORT_ENTRY
> - PortEntry[MPI26_EVENT_PCIE_TOPO_PORT_COUNT]; /*0x0C */
> + PortEntry[]; /*0x0C */
> } MPI26_EVENT_DATA_PCIE_TOPOLOGY_CHANGE_LIST,
> *PTR_MPI26_EVENT_DATA_PCIE_TOPOLOGY_CHANGE_LIST,
> Mpi26EventDataPCIeTopologyChangeList_t,
> --
> 2.39.2
>
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/6] scsi: mpt3sas: Use flexible arrays when less obviously possible
2023-07-25 16:13 ` [PATCH 2/6] scsi: mpt3sas: Use flexible arrays when less " James Seo
@ 2023-07-28 22:26 ` Kees Cook
2023-07-29 8:09 ` James Seo
0 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2023-07-28 22:26 UTC (permalink / raw)
To: James Seo
Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani,
James E.J. Bottomley, Martin K. Petersen, Gustavo A. R. Silva,
MPT-FusionLinux.pdl, linux-scsi, linux-kernel
On Tue, Jul 25, 2023 at 09:13:27AM -0700, James Seo wrote:
> These old-style 1-length variable arrays can be directly converted
> into C99 flexible array members without any further source changes
> and without any meaningful binary changes. All uses of the affected
> structs were investigated, and the existing code somehow manages to
> weather the reduced sizeof() the affected structs in every case.
Doing build comparisons here, I see a lot of binary changes. They may
be, as you say, harmless, and since you've actually got hardware then
this is a good verification of the changes, but I do wonder if this
needs more detailed commit log (or split up patches).
However, the problem I see is that this code was already doing weird
stuff with structs that appear to not have been using flex arrays
actually. With "pahole" I can see struct MPT3SAS_ADAPTER changes:
- Mpi2IOUnitPage8_t iounit_pg8; /* 3668 40 */
- Mpi2IOCPage1_t ioc_pg1_copy; /* 3708 24 */
+ Mpi2IOUnitPage8_t iounit_pg8; /* 3668 16 */
+ Mpi2IOCPage1_t ioc_pg1_copy; /* 3684 24 */
struct _MPI2_CONFIG_PAGE_IO_UNIT_8 (Mpi2IOUnitPage8_t) is in the
_middle_ of struct MPT3SAS_ADAPTER.... :|
In the earlier attempts at this conversion, it seemed that some of these
are actually fixed-size:
https://lore.kernel.org/lkml/20210202235118.GA314410@embeddedor/
I think this patch needs to be broken up into per-struct changes, so
they can be reviewed individually.
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/6] scsi: mpt3sas: Fix an outdated comment
2023-07-25 16:13 ` [PATCH 4/6] scsi: mpt3sas: Fix an outdated comment James Seo
@ 2023-07-28 22:27 ` Kees Cook
0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2023-07-28 22:27 UTC (permalink / raw)
To: James Seo
Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani,
James E.J. Bottomley, Martin K. Petersen, Gustavo A. R. Silva,
MPT-FusionLinux.pdl, linux-scsi, linux-kernel
On Tue, Jul 25, 2023 at 09:13:29AM -0700, James Seo wrote:
> May reduce confusion for users of MPI2_CONFIG_PAGE_IO_UNIT_3::GPIOVal[].
>
> Fixes: a1c4d7741323 ("scsi: mpt3sas: Replace unnecessary dynamic allocation with a static one")
> Signed-off-by: James Seo <james@equiv.tech>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/6] scsi: mpt3sas: Fix typo of "TRIGGER"
2023-07-25 16:13 ` [PATCH 5/6] scsi: mpt3sas: Fix typo of "TRIGGER" James Seo
@ 2023-07-28 22:27 ` Kees Cook
0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2023-07-28 22:27 UTC (permalink / raw)
To: James Seo
Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani,
James E.J. Bottomley, Martin K. Petersen, Gustavo A. R. Silva,
MPT-FusionLinux.pdl, linux-scsi, linux-kernel
On Tue, Jul 25, 2023 at 09:13:30AM -0700, James Seo wrote:
> Change "TIGGER" to "TRIGGER" in struct names and typedefs.
>
> Signed-off-by: James Seo <james@equiv.tech>
LoL. Yup.
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] scsi: mpt3sas: Replace a dynamic allocation with a local variable
2023-07-25 16:13 ` [PATCH 6/6] scsi: mpt3sas: Replace a dynamic allocation with a local variable James Seo
@ 2023-07-28 22:29 ` Kees Cook
2023-07-29 8:35 ` James Seo
0 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2023-07-28 22:29 UTC (permalink / raw)
To: James Seo
Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani,
James E.J. Bottomley, Martin K. Petersen, Gustavo A. R. Silva,
MPT-FusionLinux.pdl, linux-scsi, linux-kernel
On Tue, Jul 25, 2023 at 09:13:31AM -0700, James Seo wrote:
> This dynamic allocation can be replaced with a local variable.
>
> Signed-off-by: James Seo <james@equiv.tech>
> ---
> drivers/scsi/mpt3sas/mpt3sas_base.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index cd6f36094159..a32a6fa728a7 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -5361,10 +5361,9 @@ _base_update_diag_trigger_pages(struct MPT3SAS_ADAPTER *ioc)
> static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc)
> {
> Mpi2ConfigReply_t mpi_reply;
> - Mpi2SasIOUnitPage1_t *sas_iounit_pg1 = NULL;
> + Mpi2SasIOUnitPage1_t sas_iounit_pg1;
> Mpi26PCIeIOUnitPage1_t pcie_iounit_pg1;
> u16 depth;
> - int sz;
> int rc = 0;
>
> ioc->max_wideport_qd = MPT3SAS_SAS_QUEUE_DEPTH;
> @@ -5374,28 +5373,21 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc)
> if (!ioc->is_gen35_ioc)
> goto out;
> /* sas iounit page 1 */
> - sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData);
> - sas_iounit_pg1 = kzalloc(sizeof(Mpi2SasIOUnitPage1_t), GFP_KERNEL);
Hunh. So Mpi2SasIOUnitPage1_t is used without the flexarray at all?
-Kees
> - if (!sas_iounit_pg1) {
> - pr_err("%s: failure at %s:%d/%s()!\n",
> - ioc->name, __FILE__, __LINE__, __func__);
> - return rc;
> - }
> rc = mpt3sas_config_get_sas_iounit_pg1(ioc, &mpi_reply,
> - sas_iounit_pg1, sz);
> + &sas_iounit_pg1, sizeof(Mpi2SasIOUnitPage1_t));
> if (rc) {
> pr_err("%s: failure at %s:%d/%s()!\n",
> ioc->name, __FILE__, __LINE__, __func__);
> goto out;
> }
>
> - depth = le16_to_cpu(sas_iounit_pg1->SASWideMaxQueueDepth);
> + depth = le16_to_cpu(sas_iounit_pg1.SASWideMaxQueueDepth);
> ioc->max_wideport_qd = (depth ? depth : MPT3SAS_SAS_QUEUE_DEPTH);
>
> - depth = le16_to_cpu(sas_iounit_pg1->SASNarrowMaxQueueDepth);
> + depth = le16_to_cpu(sas_iounit_pg1.SASNarrowMaxQueueDepth);
> ioc->max_narrowport_qd = (depth ? depth : MPT3SAS_SAS_QUEUE_DEPTH);
>
> - depth = sas_iounit_pg1->SATAMaxQDepth;
> + depth = sas_iounit_pg1.SATAMaxQDepth;
> ioc->max_sata_qd = (depth ? depth : MPT3SAS_SATA_QUEUE_DEPTH);
>
> /* pcie iounit page 1 */
> @@ -5414,7 +5406,6 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc)
> "MaxWidePortQD: 0x%x MaxNarrowPortQD: 0x%x MaxSataQD: 0x%x MaxNvmeQD: 0x%x\n",
> ioc->max_wideport_qd, ioc->max_narrowport_qd,
> ioc->max_sata_qd, ioc->max_nvme_qd));
> - kfree(sas_iounit_pg1);
> return rc;
> }
>
> --
> 2.39.2
>
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/6] scsi: mpt3sas: Use flexible arrays when less obviously possible
2023-07-28 22:26 ` Kees Cook
@ 2023-07-29 8:09 ` James Seo
0 siblings, 0 replies; 14+ messages in thread
From: James Seo @ 2023-07-29 8:09 UTC (permalink / raw)
To: Kees Cook
Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani,
James E.J. Bottomley, Martin K. Petersen, Gustavo A. R. Silva,
MPT-FusionLinux.pdl, linux-scsi, linux-kernel
Hi, thanks for reviewing.
On Fri, Jul 28, 2023 at 03:26:57PM -0700, Kees Cook wrote:
> Doing build comparisons here, I see a lot of binary changes. They may
> be, as you say, harmless, and since you've actually got hardware then
> this is a good verification of the changes, but I do wonder if this
> needs more detailed commit log (or split up patches).
>
> However, the problem I see is that this code was already doing weird
> stuff with structs that appear to not have been using flex arrays
> actually. With "pahole" I can see struct MPT3SAS_ADAPTER changes:
>
> - Mpi2IOUnitPage8_t iounit_pg8; /* 3668 40 */
> - Mpi2IOCPage1_t ioc_pg1_copy; /* 3708 24 */
> + Mpi2IOUnitPage8_t iounit_pg8; /* 3668 16 */
> + Mpi2IOCPage1_t ioc_pg1_copy; /* 3684 24 */
>
> struct _MPI2_CONFIG_PAGE_IO_UNIT_8 (Mpi2IOUnitPage8_t) is in the
> _middle_ of struct MPT3SAS_ADAPTER.... :|
In this particular case, the flex array member of iounit_pg8 is never
used, and iounit_pg8 itself is never used outside of the function
that fetches and sets it on the per-adapter struct MPT3SAS_ADAPTER.
iounit_pg8 could probably be removed, now that I think about it.
Maybe I will.
> In the earlier attempts at this conversion, it seemed that some of these
> are actually fixed-size:
>
> https://lore.kernel.org/lkml/20210202235118.GA314410@embeddedor/
Yes, I tried to leave such terminal arrays alone. But I'll revisit
each change in this commit.
> I think this patch needs to be broken up into per-struct changes, so
> they can be reviewed individually.
Sure, I can do that. I'll resubmit this commit and the one following
(which depends on this commit) as a new series with more details.
Hopefully this will encourage the Broadcom folks who know this driver
best to chime in as well.
By the way, I noticed you've done something like this in the past to
preserve struct size for userspace, just in case:
/* MPI2_IOUNIT8_SENSOR Sensor[1]; */
union {
MPI2_IOUNIT8_SENSOR _LegacyPadding;
__DECLARE_FLEX_ARRAY(MPI2_IOUNIT8_SENSOR, Sensor);
};
I don't think userspace is a concern for us here, but would you be
more comfortable if I did this too/instead?
James
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] scsi: mpt3sas: Replace a dynamic allocation with a local variable
2023-07-28 22:29 ` Kees Cook
@ 2023-07-29 8:35 ` James Seo
0 siblings, 0 replies; 14+ messages in thread
From: James Seo @ 2023-07-29 8:35 UTC (permalink / raw)
To: Kees Cook
Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani,
James E.J. Bottomley, Martin K. Petersen, Gustavo A. R. Silva,
MPT-FusionLinux.pdl, linux-scsi, linux-kernel
On Fri, Jul 28, 2023 at 03:29:17PM -0700, Kees Cook wrote:
> On Tue, Jul 25, 2023 at 09:13:31AM -0700, James Seo wrote:
>> This dynamic allocation can be replaced with a local variable.
>>
>> Signed-off-by: James Seo <james@equiv.tech>
>> ---
>> drivers/scsi/mpt3sas/mpt3sas_base.c | 19 +++++--------------
>> 1 file changed, 5 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index cd6f36094159..a32a6fa728a7 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -5361,10 +5361,9 @@ _base_update_diag_trigger_pages(struct MPT3SAS_ADAPTER *ioc)
>> static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc)
>> {
>> Mpi2ConfigReply_t mpi_reply;
>> - Mpi2SasIOUnitPage1_t *sas_iounit_pg1 = NULL;
>> + Mpi2SasIOUnitPage1_t sas_iounit_pg1;
>> Mpi26PCIeIOUnitPage1_t pcie_iounit_pg1;
>> u16 depth;
>> - int sz;
>> int rc = 0;
>>
>> ioc->max_wideport_qd = MPT3SAS_SAS_QUEUE_DEPTH;
>> @@ -5374,28 +5373,21 @@ static int _base_assign_fw_reported_qd(struct MPT3SAS_ADAPTER *ioc)
>> if (!ioc->is_gen35_ioc)
>> goto out;
>> /* sas iounit page 1 */
>> - sz = offsetof(Mpi2SasIOUnitPage1_t, PhyData);
>> - sas_iounit_pg1 = kzalloc(sizeof(Mpi2SasIOUnitPage1_t), GFP_KERNEL);
>
> Hunh. So Mpi2SasIOUnitPage1_t is used without the flexarray at all?
>
> -Kees
You call it "dead code" and "unused struct members".
mpt3sas evidently calls it "documentation" ;)
Anyway, does this commit get your "Reviewed-by:"?
James
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-07-29 8:35 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-25 16:13 [PATCH 0/6] scsi: mpt3sas: Use flexible arrays and do a few cleanups James Seo
2023-07-25 16:13 ` [PATCH 1/6] scsi: mpt3sas: Use flexible arrays when obviously possible James Seo
2023-07-28 22:01 ` Kees Cook
2023-07-25 16:13 ` [PATCH 2/6] scsi: mpt3sas: Use flexible arrays when less " James Seo
2023-07-28 22:26 ` Kees Cook
2023-07-29 8:09 ` James Seo
2023-07-25 16:13 ` [PATCH 3/6] scsi: mpt3sas: Use struct_size() for struct size calculations James Seo
2023-07-25 16:13 ` [PATCH 4/6] scsi: mpt3sas: Fix an outdated comment James Seo
2023-07-28 22:27 ` Kees Cook
2023-07-25 16:13 ` [PATCH 5/6] scsi: mpt3sas: Fix typo of "TRIGGER" James Seo
2023-07-28 22:27 ` Kees Cook
2023-07-25 16:13 ` [PATCH 6/6] scsi: mpt3sas: Replace a dynamic allocation with a local variable James Seo
2023-07-28 22:29 ` Kees Cook
2023-07-29 8:35 ` James Seo
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).