diff mbox series

scsi: mvumi: Replace 1-element arrays with flexible array members

Message ID 20230105011143.never.569-kees@kernel.org
State New
Headers
Series scsi: mvumi: Replace 1-element arrays with flexible array members |

Commit Message

Kees Cook Jan. 5, 2023, 1:11 a.m. UTC 
  One-element arrays (and multi-element arrays being treated as
dynamically sized) are deprecated[1] and are being replaced with
flexible array members in support of the ongoing efforts to tighten the
FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.

Replace one-element arrays with flexible-array member in struct
mvumi_msg_frame, struct mvumi_rsp_frame, and struct mvumi_hs_header,
adjusting the explicit sizing calculations at the same time.

This results in no functional differences in binary output. An explicit
add is now folded into the size calculation:

│       mov    0x1070(%r14),%eax
│ -     add    $0x4,%eax
│ -     movabs $0xfffffffdc,%rbx
│ +     movabs $0xfffffffe0,%rbx
│       add    %rax,%rbx

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays

Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/scsi/mvumi.c | 4 ++--
 drivers/scsi/mvumi.h | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Gustavo A. R. Silva Jan. 5, 2023, 6:12 p.m. UTC | #1 
On Wed, Jan 04, 2023 at 05:11:50PM -0800, Kees Cook wrote:
> One-element arrays (and multi-element arrays being treated as
> dynamically sized) are deprecated[1] and are being replaced with
> flexible array members in support of the ongoing efforts to tighten the
> FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
> 
> Replace one-element arrays with flexible-array member in struct
> mvumi_msg_frame, struct mvumi_rsp_frame, and struct mvumi_hs_header,
> adjusting the explicit sizing calculations at the same time.
> 
> This results in no functional differences in binary output. An explicit
> add is now folded into the size calculation:
> 
> │       mov    0x1070(%r14),%eax
> │ -     add    $0x4,%eax
> │ -     movabs $0xfffffffdc,%rbx
> │ +     movabs $0xfffffffe0,%rbx
> │       add    %rax,%rbx
> 
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
> 
> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: linux-scsi@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Wow, those "magical" 4s seem quite elusive. It's more common to see people
using sizeof applied to the element type, like sizeof(u32).

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks!
--
Gustavo

> ---
>  drivers/scsi/mvumi.c | 4 ++--
>  drivers/scsi/mvumi.h | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
> index b3dcb8918618..60c65586f30e 100644
> --- a/drivers/scsi/mvumi.c
> +++ b/drivers/scsi/mvumi.c
> @@ -1841,7 +1841,7 @@ static enum mvumi_qc_result mvumi_send_command(struct mvumi_hba *mhba,
>  	cmd->frame->request_id = mhba->io_seq++;
>  	cmd->request_id = cmd->frame->request_id;
>  	mhba->tag_cmd[cmd->frame->tag] = cmd;
> -	frame_len = sizeof(*ib_frame) - 4 +
> +	frame_len = sizeof(*ib_frame) +
>  				ib_frame->sg_counts * sizeof(struct mvumi_sgl);
>  	if (mhba->hba_capability & HS_CAPABILITY_SUPPORT_DYN_SRC) {
>  		struct mvumi_dyn_list_entry *dle;
> @@ -2387,7 +2387,7 @@ static int mvumi_io_attach(struct mvumi_hba *mhba)
>  	struct Scsi_Host *host = mhba->shost;
>  	struct scsi_device *sdev = NULL;
>  	int ret;
> -	unsigned int max_sg = (mhba->ib_max_size + 4 -
> +	unsigned int max_sg = (mhba->ib_max_size -
>  		sizeof(struct mvumi_msg_frame)) / sizeof(struct mvumi_sgl);
>  
>  	host->irq = mhba->pdev->irq;
> diff --git a/drivers/scsi/mvumi.h b/drivers/scsi/mvumi.h
> index a88c58787b68..1306a4abf19a 100644
> --- a/drivers/scsi/mvumi.h
> +++ b/drivers/scsi/mvumi.h
> @@ -279,7 +279,7 @@ struct mvumi_msg_frame {
>  	u16 request_id;
>  	u16 reserved1;
>  	u8 cdb[MAX_COMMAND_SIZE];
> -	u32 payload[1];
> +	u32 payload[];
>  };
>  
>  /*
> @@ -294,7 +294,7 @@ struct mvumi_rsp_frame {
>  	u8 req_status;
>  	u8 rsp_flag;	/* Indicates the type of Data_Payload.*/
>  	u16 request_id;
> -	u32 payload[1];
> +	u32 payload[];
>  };
>  
>  struct mvumi_ob_data {
> @@ -380,7 +380,7 @@ struct mvumi_hs_header {
>  	u8	page_code;
>  	u8	checksum;
>  	u16	frame_length;
> -	u32	frame_content[1];
> +	u32	frame_content[];
>  };
>  
>  /*
> -- 
> 2.34.1
>
Martin K. Petersen Jan. 12, 2023, 5:11 a.m. UTC | #2 
Kees,

> One-element arrays (and multi-element arrays being treated as
> dynamically sized) are deprecated[1] and are being replaced with
> flexible array members in support of the ongoing efforts to tighten
> the FORTIFY_SOURCE routines on memcpy(), correctly instrument array
> indexing with UBSAN_BOUNDS, and to globally enable
> -fstrict-flex-arrays=3.
>
> Replace one-element arrays with flexible-array member in struct
> mvumi_msg_frame, struct mvumi_rsp_frame, and struct mvumi_hs_header,
> adjusting the explicit sizing calculations at the same time.

Applied to 6.3/scsi-staging, thanks!
Martin K. Petersen Jan. 14, 2023, 3:06 a.m. UTC | #3 
On Wed, 04 Jan 2023 17:11:50 -0800, Kees Cook wrote:

> One-element arrays (and multi-element arrays being treated as
> dynamically sized) are deprecated[1] and are being replaced with
> flexible array members in support of the ongoing efforts to tighten the
> FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
> 
> Replace one-element arrays with flexible-array member in struct
> mvumi_msg_frame, struct mvumi_rsp_frame, and struct mvumi_hs_header,
> adjusting the explicit sizing calculations at the same time.
> 
> [...]

Applied to 6.3/scsi-queue, thanks!

[1/1] scsi: mvumi: Replace 1-element arrays with flexible array members
      https://git.kernel.org/mkp/scsi/c/201e0a7c7f36
diff mbox series

Patch

diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
index b3dcb8918618..60c65586f30e 100644
--- a/drivers/scsi/mvumi.c
+++ b/drivers/scsi/mvumi.c
@@ -1841,7 +1841,7 @@  static enum mvumi_qc_result mvumi_send_command(struct mvumi_hba *mhba,
 	cmd->frame->request_id = mhba->io_seq++;
 	cmd->request_id = cmd->frame->request_id;
 	mhba->tag_cmd[cmd->frame->tag] = cmd;
-	frame_len = sizeof(*ib_frame) - 4 +
+	frame_len = sizeof(*ib_frame) +
 				ib_frame->sg_counts * sizeof(struct mvumi_sgl);
 	if (mhba->hba_capability & HS_CAPABILITY_SUPPORT_DYN_SRC) {
 		struct mvumi_dyn_list_entry *dle;
@@ -2387,7 +2387,7 @@  static int mvumi_io_attach(struct mvumi_hba *mhba)
 	struct Scsi_Host *host = mhba->shost;
 	struct scsi_device *sdev = NULL;
 	int ret;
-	unsigned int max_sg = (mhba->ib_max_size + 4 -
+	unsigned int max_sg = (mhba->ib_max_size -
 		sizeof(struct mvumi_msg_frame)) / sizeof(struct mvumi_sgl);
 
 	host->irq = mhba->pdev->irq;
diff --git a/drivers/scsi/mvumi.h b/drivers/scsi/mvumi.h
index a88c58787b68..1306a4abf19a 100644
--- a/drivers/scsi/mvumi.h
+++ b/drivers/scsi/mvumi.h
@@ -279,7 +279,7 @@  struct mvumi_msg_frame {
 	u16 request_id;
 	u16 reserved1;
 	u8 cdb[MAX_COMMAND_SIZE];
-	u32 payload[1];
+	u32 payload[];
 };
 
 /*
@@ -294,7 +294,7 @@  struct mvumi_rsp_frame {
 	u8 req_status;
 	u8 rsp_flag;	/* Indicates the type of Data_Payload.*/
 	u16 request_id;
-	u32 payload[1];
+	u32 payload[];
 };
 
 struct mvumi_ob_data {
@@ -380,7 +380,7 @@  struct mvumi_hs_header {
 	u8	page_code;
 	u8	checksum;
 	u16	frame_length;
-	u32	frame_content[1];
+	u32	frame_content[];
 };
 
 /*