* [PATCH 0/2][next] Replace one-element array with flexible-array member
@ 2022-09-22 16:52 Gustavo A. R. Silva
2022-09-22 16:53 ` [PATCH 1/2][next] scsi: hptiop: " Gustavo A. R. Silva
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2022-09-22 16:52 UTC (permalink / raw)
To: HighPoint Linux Team, James E.J. Bottomley, Martin K. Petersen
Cc: linux-scsi, linux-kernel, Gustavo A. R. Silva, linux-hardening
Hi!
This series aims to replace one-element arrays with flexible-array
members in drivers/scsi/hptiop.h
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].
Link: https://en.wikipedia.org/wiki/Flexible_array_member
Link: https://www.kernel.org/doc/html/v5.10/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Gustavo A. R. Silva (2):
scsi: hptiop: Replace one-element array with flexible-array member
scsi: hptiop: Use struct_size() helper in code related to struct
hpt_iop_request_scsi_command
drivers/scsi/hptiop.c | 9 +++------
drivers/scsi/hptiop.h | 2 +-
2 files changed, 4 insertions(+), 7 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2][next] scsi: hptiop: Replace one-element array with flexible-array member
2022-09-22 16:52 [PATCH 0/2][next] Replace one-element array with flexible-array member Gustavo A. R. Silva
@ 2022-09-22 16:53 ` Gustavo A. R. Silva
2022-09-24 6:01 ` Kees Cook
2022-09-22 16:55 ` [PATCH 2/2][next] scsi: hptiop: Use struct_size() helper in code related to struct hpt_iop_request_scsi_command Gustavo A. R. Silva
2022-09-25 17:03 ` [PATCH 0/2][next] Replace one-element array with flexible-array member Martin K. Petersen
2 siblings, 1 reply; 6+ messages in thread
From: Gustavo A. R. Silva @ 2022-09-22 16:53 UTC (permalink / raw)
To: HighPoint Linux Team, James E.J. Bottomley, Martin K. Petersen
Cc: linux-scsi, linux-kernel, Gustavo A. R. Silva, linux-hardening
One-element arrays are deprecated, and we are replacing them with flexible
array members instead. So, replace one-element array with flexible-array
member in struct hpt_iop_request_scsi_command and refactor the rest of the
code, accordingly.
The following pieces of code suggest that the one element of array sg_list
in struct hpt_iop_request_scsi_command is not taken into account when
calculating the total size for both struct hpt_iop_request_scsi_command
and the maximum number of elements sg_list will contain:
1047 req->header.size = cpu_to_le32(
1048 sizeof(struct hpt_iop_request_scsi_command)
1049 - sizeof(struct hpt_iopsg)
1050 + sg_count * sizeof(struct hpt_iopsg));
1400 req_size = sizeof(struct hpt_iop_request_scsi_command) 1401 + sizeof(struct hpt_iopsg) * (hba->max_sg_descriptors - 1);
So it's safe to replace the one-element array with a flexible-array
member and update the code above, accordingly: now we don't need to
subtract sizeof(struct hpt_iopsg) from sizeof(struct hpt_iop_request_scsi_command)
because this is implicitly done by the flex-array transformation.
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/205
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/scsi/hptiop.c | 3 +--
drivers/scsi/hptiop.h | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index f18b770626e6..cfc6546e35a6 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -1046,7 +1046,6 @@ static int hptiop_queuecommand_lck(struct scsi_cmnd *scp)
req->lun = scp->device->lun;
req->header.size = cpu_to_le32(
sizeof(struct hpt_iop_request_scsi_command)
- - sizeof(struct hpt_iopsg)
+ sg_count * sizeof(struct hpt_iopsg));
memcpy(req->cdb, scp->cmnd, sizeof(req->cdb));
@@ -1398,7 +1397,7 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
host->max_cmd_len = 16;
req_size = sizeof(struct hpt_iop_request_scsi_command)
- + sizeof(struct hpt_iopsg) * (hba->max_sg_descriptors - 1);
+ + sizeof(struct hpt_iopsg) * hba->max_sg_descriptors;
if ((req_size & 0x1f) != 0)
req_size = (req_size + 0x1f) & ~0x1f;
diff --git a/drivers/scsi/hptiop.h b/drivers/scsi/hptiop.h
index 363d5a16243f..ef2f2aca598c 100644
--- a/drivers/scsi/hptiop.h
+++ b/drivers/scsi/hptiop.h
@@ -228,7 +228,7 @@ struct hpt_iop_request_scsi_command {
u8 pad1;
u8 cdb[16];
__le32 dataxfer_length;
- struct hpt_iopsg sg_list[1];
+ struct hpt_iopsg sg_list[];
};
struct hpt_iop_request_ioctl_command {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2][next] scsi: hptiop: Use struct_size() helper in code related to struct hpt_iop_request_scsi_command
2022-09-22 16:52 [PATCH 0/2][next] Replace one-element array with flexible-array member Gustavo A. R. Silva
2022-09-22 16:53 ` [PATCH 1/2][next] scsi: hptiop: " Gustavo A. R. Silva
@ 2022-09-22 16:55 ` Gustavo A. R. Silva
2022-09-24 6:06 ` Kees Cook
2022-09-25 17:03 ` [PATCH 0/2][next] Replace one-element array with flexible-array member Martin K. Petersen
2 siblings, 1 reply; 6+ messages in thread
From: Gustavo A. R. Silva @ 2022-09-22 16:55 UTC (permalink / raw)
To: HighPoint Linux Team, James E.J. Bottomley, Martin K. Petersen
Cc: linux-scsi, linux-kernel, Gustavo A. R. Silva, linux-hardening
Prefer struct_size() over open-coded versions of idiom:
sizeof(struct-with-flex-array) + sizeof(typeof-flex-array-elements) * count
where count is the max number of items the flexible array is supposed to
contain.
Link: https://github.com/KSPP/linux/issues/160
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/scsi/hptiop.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
index cfc6546e35a6..7e8903718245 100644
--- a/drivers/scsi/hptiop.c
+++ b/drivers/scsi/hptiop.c
@@ -1044,9 +1044,7 @@ static int hptiop_queuecommand_lck(struct scsi_cmnd *scp)
req->channel = scp->device->channel;
req->target = scp->device->id;
req->lun = scp->device->lun;
- req->header.size = cpu_to_le32(
- sizeof(struct hpt_iop_request_scsi_command)
- + sg_count * sizeof(struct hpt_iopsg));
+ req->header.size = cpu_to_le32(struct_size(req, sg_list, sg_count));
memcpy(req->cdb, scp->cmnd, sizeof(req->cdb));
hba->ops->post_req(hba, _req);
@@ -1396,8 +1394,8 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
host->cmd_per_lun = le32_to_cpu(iop_config.max_requests);
host->max_cmd_len = 16;
- req_size = sizeof(struct hpt_iop_request_scsi_command)
- + sizeof(struct hpt_iopsg) * hba->max_sg_descriptors;
+ req_size = struct_size((struct hpt_iop_request_scsi_command *)0,
+ sg_list, hba->max_sg_descriptors);
if ((req_size & 0x1f) != 0)
req_size = (req_size + 0x1f) & ~0x1f;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2][next] scsi: hptiop: Replace one-element array with flexible-array member
2022-09-22 16:53 ` [PATCH 1/2][next] scsi: hptiop: " Gustavo A. R. Silva
@ 2022-09-24 6:01 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2022-09-24 6:01 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: HighPoint Linux Team, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, linux-kernel, linux-hardening
On Thu, Sep 22, 2022 at 11:53:23AM -0500, Gustavo A. R. Silva wrote:
> One-element arrays are deprecated, and we are replacing them with flexible
> array members instead. So, replace one-element array with flexible-array
> member in struct hpt_iop_request_scsi_command and refactor the rest of the
> code, accordingly.
>
> The following pieces of code suggest that the one element of array sg_list
> in struct hpt_iop_request_scsi_command is not taken into account when
> calculating the total size for both struct hpt_iop_request_scsi_command
> and the maximum number of elements sg_list will contain:
>
> 1047 req->header.size = cpu_to_le32(
> 1048 sizeof(struct hpt_iop_request_scsi_command)
> 1049 - sizeof(struct hpt_iopsg)
> 1050 + sg_count * sizeof(struct hpt_iopsg));
>
> 1400 req_size = sizeof(struct hpt_iop_request_scsi_command) 1401 + sizeof(struct hpt_iopsg) * (hba->max_sg_descriptors - 1);
Accidentally merge line above ("1401" should start a new line).
> So it's safe to replace the one-element array with a flexible-array
> member and update the code above, accordingly: now we don't need to
> subtract sizeof(struct hpt_iopsg) from sizeof(struct hpt_iop_request_scsi_command)
> because this is implicitly done by the flex-array transformation.
The only binary output change I see is from the line numbers changing
from the patch, as the argument to __might_sleep() is adjusted:
...
│ call d1 <hptiop_reset+0x74>
│ R_X86_64_PLT32 __x86_indirect_thunk_rax-0x4
│ - mov $0x434,%esi
│ + mov $0x433,%esi
│ mov $0x0,%rdi
│ R_X86_64_32S .rodata.str1.1
│ call e2 <hptiop_reset+0x85>
│ R_X86_64_PLT32 __might_sleep-0x4
...
So this looks good!
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2][next] scsi: hptiop: Use struct_size() helper in code related to struct hpt_iop_request_scsi_command
2022-09-22 16:55 ` [PATCH 2/2][next] scsi: hptiop: Use struct_size() helper in code related to struct hpt_iop_request_scsi_command Gustavo A. R. Silva
@ 2022-09-24 6:06 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2022-09-24 6:06 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: HighPoint Linux Team, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, linux-kernel, linux-hardening
On Thu, Sep 22, 2022 at 11:55:33AM -0500, Gustavo A. R. Silva wrote:
> Prefer struct_size() over open-coded versions of idiom:
>
> sizeof(struct-with-flex-array) + sizeof(typeof-flex-array-elements) * count
>
> where count is the max number of items the flexible array is supposed to
> contain.
>
> Link: https://github.com/KSPP/linux/issues/160
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Looks correct; thanks!
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2][next] Replace one-element array with flexible-array member
2022-09-22 16:52 [PATCH 0/2][next] Replace one-element array with flexible-array member Gustavo A. R. Silva
2022-09-22 16:53 ` [PATCH 1/2][next] scsi: hptiop: " Gustavo A. R. Silva
2022-09-22 16:55 ` [PATCH 2/2][next] scsi: hptiop: Use struct_size() helper in code related to struct hpt_iop_request_scsi_command Gustavo A. R. Silva
@ 2022-09-25 17:03 ` Martin K. Petersen
2 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2022-09-25 17:03 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: HighPoint Linux Team, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, linux-kernel, linux-hardening
Gustavo,
> This series aims to replace one-element arrays with flexible-array
> members in drivers/scsi/hptiop.h
Applied to 6.1/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-25 17:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-22 16:52 [PATCH 0/2][next] Replace one-element array with flexible-array member Gustavo A. R. Silva
2022-09-22 16:53 ` [PATCH 1/2][next] scsi: hptiop: " Gustavo A. R. Silva
2022-09-24 6:01 ` Kees Cook
2022-09-22 16:55 ` [PATCH 2/2][next] scsi: hptiop: Use struct_size() helper in code related to struct hpt_iop_request_scsi_command Gustavo A. R. Silva
2022-09-24 6:06 ` Kees Cook
2022-09-25 17:03 ` [PATCH 0/2][next] Replace one-element array with flexible-array member Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.