diff mbox series

[next] scsi: mpt3sas: Replace one-element array with flexible-array in struct _MPI2_CONFIG_PAGE_IO_UNIT_3

Message ID 20210202000812.GA191357@embeddedor
State New
Headers show
Series [next] scsi: mpt3sas: Replace one-element array with flexible-array in struct _MPI2_CONFIG_PAGE_IO_UNIT_3 | expand

Commit Message

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

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

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

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

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

Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 11 +----------
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  6 +++---
 2 files changed, 4 insertions(+), 13 deletions(-)

Comments

kernel test robot Feb. 2, 2021, 4:12 p.m. UTC | #1
Hi "Gustavo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on scsi/for-next]
[also build test WARNING on mkp-scsi/for-next v5.11-rc6 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Gustavo-A-R-Silva/scsi-mpt3sas-Replace-one-element-array-with-flexible-array-in-struct-_MPI2_CONFIG_PAGE_IO_UNIT_3/20210202-093055
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: riscv-randconfig-r003-20210202 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 275c6af7d7f1ed63a03d05b4484413e447133269)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/bbcda75a87ca5e689b745aa6803a9cbe349e3a68
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Gustavo-A-R-Silva/scsi-mpt3sas-Replace-one-element-array-with-flexible-array-in-struct-_MPI2_CONFIG_PAGE_IO_UNIT_3/20210202-093055
        git checkout bbcda75a87ca5e689b745aa6803a9cbe349e3a68
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/scsi/mpt3sas/mpt3sas_ctl.c:3165:14: warning: format specifies type 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
                           __func__, sz);
                                     ^~
   drivers/scsi/mpt3sas/mpt3sas_base.h:181:36: note: expanded from macro 'ioc_err'
           pr_err("%s: " fmt, (ioc)->name, ##__VA_ARGS__)
                         ~~~                 ^~~~~~~~~~~
   include/linux/printk.h:343:33: note: expanded from macro 'pr_err'
           printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
                                  ~~~     ^~~~~~~~~~~
   1 warning generated.


vim +3165 drivers/scsi/mpt3sas/mpt3sas_ctl.c

f92363d1235949 Sreekanth Reddy     2012-11-30  3125  
422630955ea348 Sreekanth Reddy     2015-11-11  3126  /**
c9df1442725953 Tomas Henzl         2019-06-14  3127   * BRM_status_show - Backup Rail Monitor Status
4beb4867f049ae Bart Van Assche     2018-06-15  3128   * @cdev: pointer to embedded class device
4beb4867f049ae Bart Van Assche     2018-06-15  3129   * @attr: ?
4beb4867f049ae Bart Van Assche     2018-06-15  3130   * @buf: the buffer returned
422630955ea348 Sreekanth Reddy     2015-11-11  3131   *
422630955ea348 Sreekanth Reddy     2015-11-11  3132   * This is number of reply queues
422630955ea348 Sreekanth Reddy     2015-11-11  3133   *
422630955ea348 Sreekanth Reddy     2015-11-11  3134   * A sysfs 'read-only' shost attribute.
422630955ea348 Sreekanth Reddy     2015-11-11  3135   */
422630955ea348 Sreekanth Reddy     2015-11-11  3136  static ssize_t
c9df1442725953 Tomas Henzl         2019-06-14  3137  BRM_status_show(struct device *cdev, struct device_attribute *attr,
422630955ea348 Sreekanth Reddy     2015-11-11  3138  	char *buf)
422630955ea348 Sreekanth Reddy     2015-11-11  3139  {
422630955ea348 Sreekanth Reddy     2015-11-11  3140  	struct Scsi_Host *shost = class_to_shost(cdev);
422630955ea348 Sreekanth Reddy     2015-11-11  3141  	struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
422630955ea348 Sreekanth Reddy     2015-11-11  3142  	Mpi2IOUnitPage3_t *io_unit_pg3 = NULL;
422630955ea348 Sreekanth Reddy     2015-11-11  3143  	Mpi2ConfigReply_t mpi_reply;
422630955ea348 Sreekanth Reddy     2015-11-11  3144  	u16 backup_rail_monitor_status = 0;
422630955ea348 Sreekanth Reddy     2015-11-11  3145  	u16 ioc_status;
bbcda75a87ca5e Gustavo A. R. Silva 2021-02-01  3146  	size_t sz;
422630955ea348 Sreekanth Reddy     2015-11-11  3147  	ssize_t rc = 0;
422630955ea348 Sreekanth Reddy     2015-11-11  3148  
422630955ea348 Sreekanth Reddy     2015-11-11  3149  	if (!ioc->is_warpdrive) {
919d8a3f3fef99 Joe Perches         2018-09-17  3150  		ioc_err(ioc, "%s: BRM attribute is only for warpdrive\n",
919d8a3f3fef99 Joe Perches         2018-09-17  3151  			__func__);
cb551b8dc079d2 Damien Le Moal      2020-07-01  3152  		return 0;
422630955ea348 Sreekanth Reddy     2015-11-11  3153  	}
08c4d550c5797d Sreekanth Reddy     2015-11-11  3154  	/* pci_access_mutex lock acquired by sysfs show path */
08c4d550c5797d Sreekanth Reddy     2015-11-11  3155  	mutex_lock(&ioc->pci_access_mutex);
0fd181456aa082 Johannes Thumshirn  2020-07-01  3156  	if (ioc->pci_error_recovery || ioc->remove_host)
0fd181456aa082 Johannes Thumshirn  2020-07-01  3157  		goto out;
422630955ea348 Sreekanth Reddy     2015-11-11  3158  
422630955ea348 Sreekanth Reddy     2015-11-11  3159  	/* allocate upto GPIOVal 36 entries */
bbcda75a87ca5e Gustavo A. R. Silva 2021-02-01  3160  	sz = struct_size(io_unit_pg3, GPIOVal, 36);
422630955ea348 Sreekanth Reddy     2015-11-11  3161  	io_unit_pg3 = kzalloc(sz, GFP_KERNEL);
422630955ea348 Sreekanth Reddy     2015-11-11  3162  	if (!io_unit_pg3) {
0fd181456aa082 Johannes Thumshirn  2020-07-01  3163  		rc = -ENOMEM;
bbcda75a87ca5e Gustavo A. R. Silva 2021-02-01  3164  		ioc_err(ioc, "%s: failed allocating memory for iounit_pg3: (%ld) bytes\n",
919d8a3f3fef99 Joe Perches         2018-09-17 @3165  			__func__, sz);
422630955ea348 Sreekanth Reddy     2015-11-11  3166  		goto out;
422630955ea348 Sreekanth Reddy     2015-11-11  3167  	}
422630955ea348 Sreekanth Reddy     2015-11-11  3168  
422630955ea348 Sreekanth Reddy     2015-11-11  3169  	if (mpt3sas_config_get_iounit_pg3(ioc, &mpi_reply, io_unit_pg3, sz) !=
422630955ea348 Sreekanth Reddy     2015-11-11  3170  	    0) {
919d8a3f3fef99 Joe Perches         2018-09-17  3171  		ioc_err(ioc, "%s: failed reading iounit_pg3\n",
422630955ea348 Sreekanth Reddy     2015-11-11  3172  			__func__);
0fd181456aa082 Johannes Thumshirn  2020-07-01  3173  		rc = -EINVAL;
422630955ea348 Sreekanth Reddy     2015-11-11  3174  		goto out;
422630955ea348 Sreekanth Reddy     2015-11-11  3175  	}
422630955ea348 Sreekanth Reddy     2015-11-11  3176  
422630955ea348 Sreekanth Reddy     2015-11-11  3177  	ioc_status = le16_to_cpu(mpi_reply.IOCStatus) & MPI2_IOCSTATUS_MASK;
422630955ea348 Sreekanth Reddy     2015-11-11  3178  	if (ioc_status != MPI2_IOCSTATUS_SUCCESS) {
919d8a3f3fef99 Joe Perches         2018-09-17  3179  		ioc_err(ioc, "%s: iounit_pg3 failed with ioc_status(0x%04x)\n",
919d8a3f3fef99 Joe Perches         2018-09-17  3180  			__func__, ioc_status);
0fd181456aa082 Johannes Thumshirn  2020-07-01  3181  		rc = -EINVAL;
422630955ea348 Sreekanth Reddy     2015-11-11  3182  		goto out;
422630955ea348 Sreekanth Reddy     2015-11-11  3183  	}
422630955ea348 Sreekanth Reddy     2015-11-11  3184  
422630955ea348 Sreekanth Reddy     2015-11-11  3185  	if (io_unit_pg3->GPIOCount < 25) {
919d8a3f3fef99 Joe Perches         2018-09-17  3186  		ioc_err(ioc, "%s: iounit_pg3->GPIOCount less than 25 entries, detected (%d) entries\n",
919d8a3f3fef99 Joe Perches         2018-09-17  3187  			__func__, io_unit_pg3->GPIOCount);
0fd181456aa082 Johannes Thumshirn  2020-07-01  3188  		rc = -EINVAL;
422630955ea348 Sreekanth Reddy     2015-11-11  3189  		goto out;
422630955ea348 Sreekanth Reddy     2015-11-11  3190  	}
422630955ea348 Sreekanth Reddy     2015-11-11  3191  
422630955ea348 Sreekanth Reddy     2015-11-11  3192  	/* BRM status is in bit zero of GPIOVal[24] */
422630955ea348 Sreekanth Reddy     2015-11-11  3193  	backup_rail_monitor_status = le16_to_cpu(io_unit_pg3->GPIOVal[24]);
422630955ea348 Sreekanth Reddy     2015-11-11  3194  	rc = snprintf(buf, PAGE_SIZE, "%d\n", (backup_rail_monitor_status & 1));
422630955ea348 Sreekanth Reddy     2015-11-11  3195  
422630955ea348 Sreekanth Reddy     2015-11-11  3196   out:
422630955ea348 Sreekanth Reddy     2015-11-11  3197  	kfree(io_unit_pg3);
08c4d550c5797d Sreekanth Reddy     2015-11-11  3198  	mutex_unlock(&ioc->pci_access_mutex);
422630955ea348 Sreekanth Reddy     2015-11-11  3199  	return rc;
422630955ea348 Sreekanth Reddy     2015-11-11  3200  }
c9df1442725953 Tomas Henzl         2019-06-14  3201  static DEVICE_ATTR_RO(BRM_status);
422630955ea348 Sreekanth Reddy     2015-11-11  3202  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

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