| Message ID | 20220427133657.55241-1-huobean@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] scsi: libsas: Fix array-bounds warnings | expand |
On 27/04/2022 14:36, Bean Huo wrote: > From: Bean Huo <beanhuo@micron.com> > > 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@micron.com> > --- > 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;
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)); 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;