diff mbox series

[2/3] scsi: libsas: introduce struct smp_rg_resp

Message ID 20220609022456.409087-3-damien.lemoal@opensource.wdc.com (mailing list archive)
State Accepted
Headers show
Series Fix compilation warnings with gcc12 | expand

Commit Message

Damien Le Moal June 9, 2022, 2:24 a.m. UTC
When compiling with gcc 12, several warnings are thrown by gcc when
compiling drivers/scsi/libsas/sas_expander.c, e.g.:

In function ‘sas_get_ex_change_count’,
    inlined from ‘sas_find_bcast_dev’ at
    drivers/scsi/libsas/sas_expander.c:1816:8:
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]
 1781 |         if (rg_resp->result != SMP_RESP_FUNC_ACC) {
      |             ~~~~~~~^~~~~~~~

This is due to the use of the struct smp_resp to aggregate all possible
response types using a union but allocating a response buffer with a
size exactly equal to the size of the response type needed. This leads
to access to fields of struct smp_resp from an allocated memory area
that is smaller than the size of struct smp_resp.

Fix this by defining struct smp_rg_resp for sas report general
responses.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/scsi/libsas/sas_expander.c | 31 +++++++++++++-----------------
 include/scsi/sas.h                 |  8 ++++++++
 2 files changed, 21 insertions(+), 18 deletions(-)

Comments

John Garry June 9, 2022, 12:07 p.m. UTC | #1
On 09/06/2022 03:24, Damien Le Moal wrote:
> When compiling with gcc 12, several warnings are thrown by gcc when
> compiling drivers/scsi/libsas/sas_expander.c, e.g.:
> 
> In function ‘sas_get_ex_change_count’,
>      inlined from ‘sas_find_bcast_dev’ at
>      drivers/scsi/libsas/sas_expander.c:1816:8:
> 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]
>   1781 |         if (rg_resp->result != SMP_RESP_FUNC_ACC) {
>        |             ~~~~~~~^~~~~~~~
> 
> This is due to the use of the struct smp_resp to aggregate all possible
> response types using a union but allocating a response buffer with a
> size exactly equal to the size of the response type needed. This leads
> to access to fields of struct smp_resp from an allocated memory area
> that is smaller than the size of struct smp_resp.
> 
> Fix this by defining struct smp_rg_resp for sas report general
> responses.
> 
> Signed-off-by: Damien Le Moal<damien.lemoal@opensource.wdc.com>

Reviewed-by: John Garry <john.garry@huawei.com>
diff mbox series

Patch

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index fb998a8a7d3b..78a38980636e 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -429,27 +429,14 @@  static int sas_expander_discover(struct domain_device *dev)
 
 #define MAX_EXPANDER_PHYS 128
 
-static void ex_assign_report_general(struct domain_device *dev,
-					    struct smp_resp *resp)
-{
-	struct report_general_resp *rg = &resp->rg;
-
-	dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
-	dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
-	dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
-	dev->ex_dev.t2t_supp = rg->t2t_supp;
-	dev->ex_dev.conf_route_table = rg->conf_route_table;
-	dev->ex_dev.configuring = rg->configuring;
-	memcpy(dev->ex_dev.enclosure_logical_id, rg->enclosure_logical_id, 8);
-}
-
 #define RG_REQ_SIZE   8
-#define RG_RESP_SIZE 32
+#define RG_RESP_SIZE  sizeof(struct smp_rg_resp)
 
 static int sas_ex_general(struct domain_device *dev)
 {
 	u8 *rg_req;
-	struct smp_resp *rg_resp;
+	struct smp_rg_resp *rg_resp;
+	struct report_general_resp *rg;
 	int res;
 	int i;
 
@@ -480,7 +467,15 @@  static int sas_ex_general(struct domain_device *dev)
 			goto out;
 		}
 
-		ex_assign_report_general(dev, rg_resp);
+		rg = &rg_resp->rg;
+		dev->ex_dev.ex_change_count = be16_to_cpu(rg->change_count);
+		dev->ex_dev.max_route_indexes = be16_to_cpu(rg->route_indexes);
+		dev->ex_dev.num_phys = min(rg->num_phys, (u8)MAX_EXPANDER_PHYS);
+		dev->ex_dev.t2t_supp = rg->t2t_supp;
+		dev->ex_dev.conf_route_table = rg->conf_route_table;
+		dev->ex_dev.configuring = rg->configuring;
+		memcpy(dev->ex_dev.enclosure_logical_id,
+		       rg->enclosure_logical_id, 8);
 
 		if (dev->ex_dev.configuring) {
 			pr_debug("RG: ex %016llx self-configuring...\n",
@@ -1756,7 +1751,7 @@  static int sas_get_ex_change_count(struct domain_device *dev, int *ecc)
 {
 	int res;
 	u8  *rg_req;
-	struct smp_resp  *rg_resp;
+	struct smp_rg_resp  *rg_resp;
 
 	rg_req = alloc_smp_req(RG_REQ_SIZE);
 	if (!rg_req)
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index b3ee9bd63277..a8f9743ed6fc 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -696,6 +696,14 @@  struct report_phy_sata_resp {
 #error "Bitfield order not defined!"
 #endif
 
+struct smp_rg_resp {
+	u8    frame_type;
+	u8    function;
+	u8    result;
+	u8    reserved;
+	struct report_general_resp rg;
+} __attribute__ ((packed));
+
 struct smp_disc_resp {
 	u8    frame_type;
 	u8    function;