| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Y7cNSXnO8yBY5OP8@work>
Date: Thu, 5 Jan 2023 11:47:53 -0600
From: "Gustavo A. R. Silva" <gustavoars@...nel.org>
To: Kees Cook <keescook@...omium.org>
Cc: Adam Radford <aradford@...il.com>,
"James E.J. Bottomley" <jejb@...ux.ibm.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH] scsi: 3w-sas: Replace 1-element arrays with flexible
array members
On Wed, Jan 04, 2023 at 04:48:01PM -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 TW_Ioctl_Buf_Apache
> and TW_Param_Apache, adjusting the explicit sizing calculations at the
> same time.
>
> This results in no differences in binary output.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>
> Cc: Adam Radford <aradford@...il.com>
> Cc: "James E.J. Bottomley" <jejb@...ux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@...cle.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>
> Cc: linux-scsi@...r.kernel.org
> Signed-off-by: Kees Cook <keescook@...omium.org>
This is great. :) It's quite similar to this other commit:
44c5027bb5c8 ("scsi: 3w-9xxx: Use flexible array members to avoid struct padding")
Reviewed-by: Gustavo A. R. Silva <gustavoars@...nel.org>
Thanks!
--
Gustavo
> ---
> drivers/scsi/3w-sas.c | 12 ++++++------
> drivers/scsi/3w-sas.h | 4 ++--
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
> index 3ebe66151dcb..f41c93454f0c 100644
> --- a/drivers/scsi/3w-sas.c
> +++ b/drivers/scsi/3w-sas.c
> @@ -690,7 +690,7 @@ static void twl_load_sgl(TW_Device_Extension *tw_dev, TW_Command_Full *full_comm
> newcommand->request_id__lunl =
> cpu_to_le16(TW_REQ_LUN_IN(TW_LUN_OUT(newcommand->request_id__lunl), request_id));
> if (length) {
> - newcommand->sg_list[0].address = TW_CPU_TO_SGL(dma_handle + sizeof(TW_Ioctl_Buf_Apache) - 1);
> + newcommand->sg_list[0].address = TW_CPU_TO_SGL(dma_handle + sizeof(TW_Ioctl_Buf_Apache));
> newcommand->sg_list[0].length = TW_CPU_TO_SGL(length);
> }
> newcommand->sgl_entries__lunh =
> @@ -702,7 +702,7 @@ static void twl_load_sgl(TW_Device_Extension *tw_dev, TW_Command_Full *full_comm
> if (TW_SGL_OUT(oldcommand->opcode__sgloffset)) {
> /* Load the sg list */
> sgl = (TW_SG_Entry_ISO *)((u32 *)oldcommand+oldcommand->size - (sizeof(TW_SG_Entry_ISO)/4) + pae + (sizeof(dma_addr_t) > 4 ? 1 : 0));
> - sgl->address = TW_CPU_TO_SGL(dma_handle + sizeof(TW_Ioctl_Buf_Apache) - 1);
> + sgl->address = TW_CPU_TO_SGL(dma_handle + sizeof(TW_Ioctl_Buf_Apache));
> sgl->length = TW_CPU_TO_SGL(length);
> oldcommand->size += pae;
> oldcommand->size += sizeof(dma_addr_t) > 4 ? 1 : 0;
> @@ -748,7 +748,7 @@ static long twl_chrdev_ioctl(struct file *file, unsigned int cmd, unsigned long
> data_buffer_length_adjusted = (driver_command.buffer_length + 511) & ~511;
>
> /* Now allocate ioctl buf memory */
> - cpu_addr = dma_alloc_coherent(&tw_dev->tw_pci_dev->dev, data_buffer_length_adjusted+sizeof(TW_Ioctl_Buf_Apache) - 1, &dma_handle, GFP_KERNEL);
> + cpu_addr = dma_alloc_coherent(&tw_dev->tw_pci_dev->dev, data_buffer_length_adjusted + sizeof(TW_Ioctl_Buf_Apache), &dma_handle, GFP_KERNEL);
> if (!cpu_addr) {
> retval = -ENOMEM;
> goto out2;
> @@ -757,7 +757,7 @@ static long twl_chrdev_ioctl(struct file *file, unsigned int cmd, unsigned long
> tw_ioctl = (TW_Ioctl_Buf_Apache *)cpu_addr;
>
> /* Now copy down the entire ioctl */
> - if (copy_from_user(tw_ioctl, argp, driver_command.buffer_length + sizeof(TW_Ioctl_Buf_Apache) - 1))
> + if (copy_from_user(tw_ioctl, argp, driver_command.buffer_length + sizeof(TW_Ioctl_Buf_Apache)))
> goto out3;
>
> /* See which ioctl we are doing */
> @@ -815,11 +815,11 @@ static long twl_chrdev_ioctl(struct file *file, unsigned int cmd, unsigned long
> }
>
> /* Now copy the entire response to userspace */
> - if (copy_to_user(argp, tw_ioctl, sizeof(TW_Ioctl_Buf_Apache) + driver_command.buffer_length - 1) == 0)
> + if (copy_to_user(argp, tw_ioctl, sizeof(TW_Ioctl_Buf_Apache) + driver_command.buffer_length) == 0)
> retval = 0;
> out3:
> /* Now free ioctl buf memory */
> - dma_free_coherent(&tw_dev->tw_pci_dev->dev, data_buffer_length_adjusted+sizeof(TW_Ioctl_Buf_Apache) - 1, cpu_addr, dma_handle);
> + dma_free_coherent(&tw_dev->tw_pci_dev->dev, data_buffer_length_adjusted + sizeof(TW_Ioctl_Buf_Apache), cpu_addr, dma_handle);
> out2:
> mutex_unlock(&tw_dev->ioctl_lock);
> out:
> diff --git a/drivers/scsi/3w-sas.h b/drivers/scsi/3w-sas.h
> index b0508039a280..096dec29e2ac 100644
> --- a/drivers/scsi/3w-sas.h
> +++ b/drivers/scsi/3w-sas.h
> @@ -335,7 +335,7 @@ typedef struct TAG_TW_Ioctl_Apache {
> TW_Ioctl_Driver_Command driver_command;
> char padding[488];
> TW_Command_Full firmware_command;
> - char data_buffer[1];
> + char data_buffer[];
> } TW_Ioctl_Buf_Apache;
>
> /* GetParam descriptor */
> @@ -344,7 +344,7 @@ typedef struct {
> unsigned short parameter_id;
> unsigned short parameter_size_bytes;
> unsigned short actual_parameter_size_bytes;
> - unsigned char data[1];
> + unsigned char data[];
> } TW_Param_Apache;
>
> /* Compatibility information structure */
> --
> 2.34.1
>
Powered by blists - more mailing lists