Re: [PATCH v1] scsi: libsas: Fix array-bounds warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/04/2022 14:36, Bean Huo wrote:
From: Bean Huo <beanhuo@xxxxxxxxxx>

Use the latest GCC will show below array-bounds warning:

Which version exactly?


drivers/scsi/libsas/sas_expander.c:1697:39: warning: array subscript ‘struct

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/libsas/sas_expander.c?h=v5.18-rc4#n1679 is a '}'

What baseline do you use?

smp_resp[0]’ is partly outside array bounds of ‘unsigned char[56]’ [-Warray-bounds]

I guess that the compiler is getting upset that we're only allocating 32 bytes for a struct which is 56 bytes in size.

...
drivers/scsi/libsas/sas_expander.c:1781:20: warning: array subscript ‘struct
smp_resp[0]’ is partly outside array bounds of ‘unsigned char[32]’ [-Warray-bounds]
...
rivers/scsi/libsas/sas_expander.c:1786:39: warning: array subscript ‘struct
smp_resp[0]’ is partly outside array bounds of ‘unsigned char[32]’ [-Warray-bounds]
...
drivers/scsi/libsas/sas_expander.c:476:35: warning: array subscript ‘struct
smp_resp[0]’ is partly outside array bounds of ‘unsigned char[32]’ [-Warray-bounds]
...
drivers/scsi/libsas/sas_expander.c:479:38: warning: array subscript ‘struct
smp_resp[0]’ is partly outside array bounds of ‘unsigned char[32]’ [-Warray-bounds]

This patch aims to fix these warnings by directly using struct sizes instead of
macro definitions.

Signed-off-by: Bean Huo <beanhuo@xxxxxxxxxx>
---
  drivers/scsi/libsas/sas_expander.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 260e735d06fa..ac6d9be358c5 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -457,7 +457,7 @@ static int sas_ex_general(struct domain_device *dev)
  	if (!rg_req)
  		return -ENOMEM;
- rg_resp = alloc_smp_resp(RG_RESP_SIZE);
+	rg_resp = alloc_smp_resp(sizeof(struct smp_resp));

I'm thinking that it's better to have something like:

struct smp_resp_hdr {
	u8    frame_type;
	u8    function;
	u8    result;
	u8    reserved;
};

struct smp_resp {
	union {
		struct report_general_resp  rg;
		struct discover_resp        disc;
		struct report_phy_sata_resp rps;
	};
} __attribute__ ((packed));


struct report_general_resp {
	struct smp_resp_hdr hdr;
	__be16  change_count;
	__be16  route_indexes;
	...
};

or even also get rid of struct smp_resp holder. Sorry if this is more than you bargained for, but I don't mind helping.

Thanks,
John

  	if (!rg_resp) {
  		kfree(rg_req);
  		return -ENOMEM;
@@ -1688,7 +1688,7 @@ static int sas_get_phy_change_count(struct domain_device *dev,
  	int res;
  	struct smp_resp *disc_resp;
- disc_resp = alloc_smp_resp(DISCOVER_RESP_SIZE);
+	disc_resp = alloc_smp_resp(sizeof(struct smp_resp));
  	if (!disc_resp)
  		return -ENOMEM;
@@ -1766,7 +1766,7 @@ static int sas_get_ex_change_count(struct domain_device *dev, int *ecc)
  	if (!rg_req)
  		return -ENOMEM;
- rg_resp = alloc_smp_resp(RG_RESP_SIZE);
+	rg_resp = alloc_smp_resp(sizeof(struct smp_resp));
  	if (!rg_resp) {
  		kfree(rg_req);
  		return -ENOMEM;




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux