* [PATCH v3 0/6] Replace one-element arrays with flexible-array members
@ 2022-08-15 21:35 Gustavo A. R. Silva
2022-08-15 21:40 ` [PATCH v3 1/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP Gustavo A. R. Silva
` (8 more replies)
0 siblings, 9 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-15 21:35 UTC (permalink / raw)
To: megaraidlinux.pdl, linux-scsi, linux-kernel
Cc: Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, Kees Cook, Gustavo A. R. Silva,
linux-hardening
Hi!
This series aims to replace one-element arrays with flexible-array
members in drivers/scsi/megaraid/
I followed the below steps in order to verify the changes don't
significally impact the code (.text) section generated by the compiler,
for each object file involved:
1. Prepare the build with the following settings and configurations:
linux$ KBF="KBUILD_BUILD_TIMESTAMP=1970-01-01 KBUILD_BUILD_USER=user
KBUILD_BUILD_HOST=host KBUILD_BUILD_VERSION=1"
linux$ make $KBF allyesconfig
linux$ ./scripts/config -d GCOV_KERNEL -d KCOV -d GCC_PLUGINS \
-d IKHEADERS -d KASAN -d UBSAN \
-d DEBUG_INFO_NONE \
-e DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
linux$ make $KBF olddefconfig
2. Build drivers/scsi/megaraid/ with the same settings and configurations
as in Step 1, and copy the generated object files in directory before/
linux$ make -j128 $KBF drivers/scsi/megaraid/
linux$ mkdir -p before
linux$ cp drivers/scsi/megaraid/*.o before/
3. Implement all the needed changes and create the patch series. In this
case, six patches.
linux$ vi code.c
...do the magic :)
linux$ git format-patch ...all the rest
4. Apply a patch at a time (of the previously created series) and, after
applying EACH patch, build (as in Step 2) drivers/scsi/megaraid/ and
copy the generated object files in directory after/
5. Compare the code section (.text) of each before/file.o and
after/file.o. I use the following bash script:
compare.sh:
ARGS="--disassemble --demangle --reloc --no-show-raw-insn --section=.text"
for i in $(cd before && echo *.o); do
echo $i
diff -u <(objdump $ARGS before/$i | sed "0,/^Disassembly/d") \
<(objdump $ARGS after/$i | sed "0,/^Disassembly/d")
done
linux$ ./compare.sh > code_comparison.diff
6. Open the code_comparison.diff file from the example above, look for
any differences that might show up and analyze them in order to
determine their impact, and what (if something) should be changed
or study further.
The above process (code section comparison of object files) is based on
this[0] blog post by Kees Cook. The compiler used to build the code was
GCC-12.
In this series I only found the following sorts of differences in files
megaraid_sas.o and megaraid_sas_base.o:
...
...@@ -7094,24 +7094,24 @@
6302: movq $0x0,0x1e20(%rbx)
630d: test %r15,%r15
6310: je 6316 <megasas_aen_polling+0x56>
- 6312: R_X86_64_PC32 .text.unlikely+0x3ae3
+ 6312: R_X86_64_PC32 .text.unlikely+0x3ae0
6316: mov 0x0(%rip),%eax # 631c <megasas_aen_polling+0x5c>
6318: R_X86_64_PC32 event_log_level-0x4
631c: mov 0xc(%r15),%r14d
6320: lea 0x2(%rax),%edx
6323: cmp $0x6,%edx
6326: ja 632c <megasas_aen_polling+0x6c>
- 6328: R_X86_64_PC32 .text.unlikely+0x3ac3
+ 6328: R_X86_64_PC32 .text.unlikely+0x3ac0
632c: mov %r14d,%edx
632f: sar $0x18,%edx
6332: mov %edx,%ecx
6334: cmp %eax,%edx
6336: jge 633c <megasas_aen_polling+0x7c>
- 6338: R_X86_64_PC32 .text.unlikely+0x399c
+ 6338: R_X86_64_PC32 .text.unlikely+0x3999
...
All of them have to do with the relocation of symbols in the
.text.unlikely subsection and they don't seem to be of any actual
relevance. So, we can safely ignore them.
Also, notice there is an open issue in bugzilla.kernel.org [1] that's
seems could be fixed by this series. :)
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 [2].
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://github.com/KSPP/linux/issues/109
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215943 [1]
Link: https://reviews.llvm.org/D126864 [2]
Thanks
[0] https://outflux.net/blog/archives/2022/06/24/finding-binary-differences/
Changes in v3:
- Split the struct_size() changes into a couple of separate patches.
- Use objdump to compare the code (.text) sections of the object
files before and after the changes.
- Modify MR_FW_RAID_MAP_ALL and MR_DRV_RAID_MAP_ALL structures. Change
suggested by Kees Cook.
Changes in v2:
- Revert changes in struct MR_FW_RAID_MAP_ALL.
Gustavo A. R. Silva (6):
scsi: megaraid_sas: Replace one-element array with flexible-array
member in MR_FW_RAID_MAP
scsi: megaraid_sas: Replace one-element array with flexible-array
member in MR_FW_RAID_MAP_DYNAMIC
scsi: megaraid_sas: Replace one-element array with flexible-array
member in MR_DRV_RAID_MAP
scsi: megaraid_sas: Replace one-element array with flexible-array
member in MR_PD_CFG_SEQ_NUM_SYNC
scsi: megaraid_sas: Use struct_size() in code related to struct
MR_FW_RAID_MAP
scsi: megaraid_sas: Use struct_size() in code related to struct
MR_PD_CFG_SEQ_NUM_SYNC
drivers/scsi/megaraid/megaraid_sas_base.c | 20 ++++++++++----------
drivers/scsi/megaraid/megaraid_sas_fp.c | 6 +++---
drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 +-
drivers/scsi/megaraid/megaraid_sas_fusion.h | 12 ++++++------
4 files changed, 20 insertions(+), 20 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
@ 2022-08-15 21:40 ` Gustavo A. R. Silva
2022-08-15 21:42 ` [PATCH v3 2/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP_DYNAMIC Gustavo A. R. Silva
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-15 21:40 UTC (permalink / raw)
To: megaraidlinux.pdl, linux-scsi, linux-kernel
Cc: Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, Kees Cook, 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 MR_DRV_RAID_MAP and refactor the
the rest of the code accordingly.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy().
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Enhanced-by: Kees Cook <keescook@chromium.org> # Change in struct MR_FW_RAID_MAP_ALL
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v3:
- Modify MR_FW_RAID_MAP_ALL structures.
- Revert use of struct_size() helper.
Changes in v2:
- Update Subject line and changelog text.
drivers/scsi/megaraid/megaraid_sas_base.c | 2 +-
drivers/scsi/megaraid/megaraid_sas_fp.c | 2 +-
drivers/scsi/megaraid/megaraid_sas_fusion.h | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index a3e117a4b8e7..c06a346a3eaf 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5159,7 +5159,7 @@ static void megasas_update_ext_vd_details(struct megasas_instance *instance)
} else {
fusion->old_map_sz = sizeof(struct MR_FW_RAID_MAP) +
(sizeof(struct MR_LD_SPAN_MAP) *
- (instance->fw_supported_vd_count - 1));
+ instance->fw_supported_vd_count);
fusion->new_map_sz = sizeof(struct MR_FW_RAID_MAP_EXT);
fusion->max_map_sz =
diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c b/drivers/scsi/megaraid/megaraid_sas_fp.c
index 83f69c33b01a..b2ec7a3f7650 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fp.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
@@ -327,7 +327,7 @@ u8 MR_ValidateMapInfo(struct megasas_instance *instance, u64 map_id)
expected_size = sizeof(struct MR_FW_RAID_MAP_EXT);
else
expected_size =
- (sizeof(struct MR_FW_RAID_MAP) - sizeof(struct MR_LD_SPAN_MAP) +
+ (sizeof(struct MR_FW_RAID_MAP) +
(sizeof(struct MR_LD_SPAN_MAP) * le16_to_cpu(pDrvRaidMap->ldCount)));
if (le32_to_cpu(pDrvRaidMap->totalSize) != expected_size) {
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h b/drivers/scsi/megaraid/megaraid_sas_fusion.h
index ce84f811e5e1..5530c233fccb 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.h
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h
@@ -942,7 +942,7 @@ struct MR_FW_RAID_MAP {
u8 reserved2[7];
struct MR_ARRAY_INFO arMapInfo[MAX_RAIDMAP_ARRAYS];
struct MR_DEV_HANDLE_INFO devHndlInfo[MAX_RAIDMAP_PHYSICAL_DEVICES];
- struct MR_LD_SPAN_MAP ldSpanMap[1];
+ struct MR_LD_SPAN_MAP ldSpanMap[];
};
struct IO_REQUEST_INFO {
@@ -1148,7 +1148,7 @@ typedef struct LOG_BLOCK_SPAN_INFO {
struct MR_FW_RAID_MAP_ALL {
struct MR_FW_RAID_MAP raidMap;
- struct MR_LD_SPAN_MAP ldSpanMap[MAX_LOGICAL_DRIVES - 1];
+ struct MR_LD_SPAN_MAP ldSpanMap[MAX_LOGICAL_DRIVES];
} __attribute__ ((packed));
struct MR_DRV_RAID_MAP {
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP_DYNAMIC
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
2022-08-15 21:40 ` [PATCH v3 1/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP Gustavo A. R. Silva
@ 2022-08-15 21:42 ` Gustavo A. R. Silva
2022-08-15 21:46 ` [PATCH v3 3/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_DRV_RAID_MAP Gustavo A. R. Silva
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-15 21:42 UTC (permalink / raw)
To: megaraidlinux.pdl, linux-scsi, linux-kernel
Cc: Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, Kees Cook, 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 MR_FW_RAID_MAP_DYNAMIC.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy().
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v3:
- None.
Changes in v2:
- None.
drivers/scsi/megaraid/megaraid_sas_fusion.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h b/drivers/scsi/megaraid/megaraid_sas_fusion.h
index 5530c233fccb..66e13b74fcdc 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.h
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h
@@ -1053,7 +1053,7 @@ struct MR_FW_RAID_MAP_DYNAMIC {
struct MR_RAID_MAP_DESC_TABLE
raid_map_desc_table[RAID_MAP_DESC_TYPE_COUNT];
/* Variable Size buffer containing all data */
- u32 raid_map_desc_data[1];
+ u32 raid_map_desc_data[];
}; /* Dynamicaly sized RAID MAp structure */
#define IEEE_SGE_FLAGS_ADDR_MASK (0x03)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_DRV_RAID_MAP
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
2022-08-15 21:40 ` [PATCH v3 1/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP Gustavo A. R. Silva
2022-08-15 21:42 ` [PATCH v3 2/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP_DYNAMIC Gustavo A. R. Silva
@ 2022-08-15 21:46 ` Gustavo A. R. Silva
2022-08-15 21:49 ` [PATCH v3 4/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_PD_CFG_SEQ_NUM_SYNC Gustavo A. R. Silva
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-15 21:46 UTC (permalink / raw)
To: megaraidlinux.pdl, linux-scsi, linux-kernel
Cc: Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, Kees Cook, 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 MR_DRV_RAID_MAP and refactor the
code accordingly.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy().
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Enhanced-by: Kees Cook <keescook@chromium.org> # Change in struct MR_DRV_RAID_MAP_ALL
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v3:
- Revert use of flex_array_size() helper.
- Modify MR_DRV_RAID_MAP_ALL structure.
Changes in v2:
- None.
drivers/scsi/megaraid/megaraid_sas_fusion.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h b/drivers/scsi/megaraid/megaraid_sas_fusion.h
index 66e13b74fcdc..df92d4369e04 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.h
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h
@@ -1182,7 +1182,7 @@ struct MR_DRV_RAID_MAP {
devHndlInfo[MAX_RAIDMAP_PHYSICAL_DEVICES_DYN];
u16 ldTgtIdToLd[MAX_LOGICAL_DRIVES_DYN];
struct MR_ARRAY_INFO arMapInfo[MAX_API_ARRAYS_DYN];
- struct MR_LD_SPAN_MAP ldSpanMap[1];
+ struct MR_LD_SPAN_MAP ldSpanMap[];
};
@@ -1193,7 +1193,7 @@ struct MR_DRV_RAID_MAP {
struct MR_DRV_RAID_MAP_ALL {
struct MR_DRV_RAID_MAP raidMap;
- struct MR_LD_SPAN_MAP ldSpanMap[MAX_LOGICAL_DRIVES_DYN - 1];
+ struct MR_LD_SPAN_MAP ldSpanMap[MAX_LOGICAL_DRIVES_DYN];
} __packed;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_PD_CFG_SEQ_NUM_SYNC
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
` (2 preceding siblings ...)
2022-08-15 21:46 ` [PATCH v3 3/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_DRV_RAID_MAP Gustavo A. R. Silva
@ 2022-08-15 21:49 ` Gustavo A. R. Silva
2022-08-15 21:51 ` [PATCH v3 5/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_FW_RAID_MAP Gustavo A. R. Silva
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-15 21:49 UTC (permalink / raw)
To: megaraidlinux.pdl, linux-scsi, linux-kernel
Cc: Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, Kees Cook, 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 MR_PD_CFG_SEQ_NUM_SYNC and refactor
the rest of the code accordingly.
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 [0].
Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Link: Link: https://reviews.llvm.org/D126864 [0]
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v3:
- Revert use of struct_size() helper.
Changes in v2:
- None.
drivers/scsi/megaraid/megaraid_sas_base.c | 4 ++--
drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 +-
drivers/scsi/megaraid/megaraid_sas_fusion.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index c06a346a3eaf..2dabe0b4823e 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5795,7 +5795,7 @@ megasas_setup_jbod_map(struct megasas_instance *instance)
u32 pd_seq_map_sz;
pd_seq_map_sz = sizeof(struct MR_PD_CFG_SEQ_NUM_SYNC) +
- (sizeof(struct MR_PD_CFG_SEQ) * (MAX_PHYSICAL_DEVICES - 1));
+ (sizeof(struct MR_PD_CFG_SEQ) * MAX_PHYSICAL_DEVICES);
instance->use_seqnum_jbod_fp =
instance->support_seqnum_jbod_fp;
@@ -8048,7 +8048,7 @@ static void megasas_detach_one(struct pci_dev *pdev)
megasas_release_fusion(instance);
pd_seq_map_sz = sizeof(struct MR_PD_CFG_SEQ_NUM_SYNC) +
(sizeof(struct MR_PD_CFG_SEQ) *
- (MAX_PHYSICAL_DEVICES - 1));
+ MAX_PHYSICAL_DEVICES);
for (i = 0; i < 2 ; i++) {
if (fusion->ld_map[i])
dma_free_coherent(&instance->pdev->dev,
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index e48d4261d0bc..f17ec83f7c98 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1310,7 +1310,7 @@ megasas_sync_pd_seq_num(struct megasas_instance *instance, bool pend) {
pd_sync = (void *)fusion->pd_seq_sync[(instance->pd_seq_map_id & 1)];
pd_seq_h = fusion->pd_seq_phys[(instance->pd_seq_map_id & 1)];
- pd_seq_map_sz = struct_size(pd_sync, seq, MAX_PHYSICAL_DEVICES - 1);
+ pd_seq_map_sz = struct_size(pd_sync, seq, MAX_PHYSICAL_DEVICES);
cmd = megasas_get_cmd(instance);
if (!cmd) {
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h b/drivers/scsi/megaraid/megaraid_sas_fusion.h
index df92d4369e04..49e9a9048ee7 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.h
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h
@@ -1249,7 +1249,7 @@ struct MR_PD_CFG_SEQ {
struct MR_PD_CFG_SEQ_NUM_SYNC {
__le32 size;
__le32 count;
- struct MR_PD_CFG_SEQ seq[1];
+ struct MR_PD_CFG_SEQ seq[];
} __packed;
/* stream detection */
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 5/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_FW_RAID_MAP
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
` (3 preceding siblings ...)
2022-08-15 21:49 ` [PATCH v3 4/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_PD_CFG_SEQ_NUM_SYNC Gustavo A. R. Silva
@ 2022-08-15 21:51 ` Gustavo A. R. Silva
2022-08-15 21:52 ` [PATCH v3 6/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_PD_CFG_SEQ_NUM_SYNC Gustavo A. R. Silva
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-15 21:51 UTC (permalink / raw)
To: megaraidlinux.pdl, linux-scsi, linux-kernel
Cc: Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, Kees Cook, Gustavo A. R. Silva,
linux-hardening
Prefer struct_size() over open-coded versions of idiom:
sizeof(struct-with-flex-array) + sizeof(type-of-flex-array) * count
where count is the max number of items the flexible array is supposed to
have.
Link: https://github.com/KSPP/linux/issues/160
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v3:
- This patch is new in the series.
drivers/scsi/megaraid/megaraid_sas_base.c | 6 +++---
drivers/scsi/megaraid/megaraid_sas_fp.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 2dabe0b4823e..09eef1bef430 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5157,9 +5157,9 @@ static void megasas_update_ext_vd_details(struct megasas_instance *instance)
fusion->current_map_sz = ventura_map_sz;
fusion->max_map_sz = ventura_map_sz;
} else {
- fusion->old_map_sz = sizeof(struct MR_FW_RAID_MAP) +
- (sizeof(struct MR_LD_SPAN_MAP) *
- instance->fw_supported_vd_count);
+ fusion->old_map_sz =
+ struct_size((struct MR_FW_RAID_MAP *)0, ldSpanMap,
+ instance->fw_supported_vd_count);
fusion->new_map_sz = sizeof(struct MR_FW_RAID_MAP_EXT);
fusion->max_map_sz =
diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c b/drivers/scsi/megaraid/megaraid_sas_fp.c
index b2ec7a3f7650..da1cad1ee123 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fp.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
@@ -326,9 +326,9 @@ u8 MR_ValidateMapInfo(struct megasas_instance *instance, u64 map_id)
else if (instance->supportmax256vd)
expected_size = sizeof(struct MR_FW_RAID_MAP_EXT);
else
- expected_size =
- (sizeof(struct MR_FW_RAID_MAP) +
- (sizeof(struct MR_LD_SPAN_MAP) * le16_to_cpu(pDrvRaidMap->ldCount)));
+ expected_size = struct_size((struct MR_FW_RAID_MAP *)0,
+ ldSpanMap,
+ le16_to_cpu(pDrvRaidMap->ldCount));
if (le32_to_cpu(pDrvRaidMap->totalSize) != expected_size) {
dev_dbg(&instance->pdev->dev, "megasas: map info structure size 0x%x",
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 6/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_PD_CFG_SEQ_NUM_SYNC
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
` (4 preceding siblings ...)
2022-08-15 21:51 ` [PATCH v3 5/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_FW_RAID_MAP Gustavo A. R. Silva
@ 2022-08-15 21:52 ` Gustavo A. R. Silva
2022-08-16 19:22 ` [PATCH v3 0/6] Replace one-element arrays with flexible-array members Kees Cook
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-15 21:52 UTC (permalink / raw)
To: megaraidlinux.pdl, linux-scsi, linux-kernel
Cc: Kashyap Desai, Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, Kees Cook, Gustavo A. R. Silva,
linux-hardening
Prefer struct_size() over open-coded versions of idiom:
sizeof(struct-with-flex-array) + sizeof(type-of-flex-array) * count
where count is the max number of items the flexible array is supposed to
have.
Link: https://github.com/KSPP/linux/issues/160
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v3:
- This patch is new in the series.
drivers/scsi/megaraid/megaraid_sas_base.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 09eef1bef430..d57cb787db0b 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -5792,10 +5792,10 @@ megasas_setup_jbod_map(struct megasas_instance *instance)
{
int i;
struct fusion_context *fusion = instance->ctrl_context;
- u32 pd_seq_map_sz;
+ size_t pd_seq_map_sz;
- pd_seq_map_sz = sizeof(struct MR_PD_CFG_SEQ_NUM_SYNC) +
- (sizeof(struct MR_PD_CFG_SEQ) * MAX_PHYSICAL_DEVICES);
+ pd_seq_map_sz = struct_size((struct MR_PD_CFG_SEQ_NUM_SYNC *)0, seq,
+ MAX_PHYSICAL_DEVICES);
instance->use_seqnum_jbod_fp =
instance->support_seqnum_jbod_fp;
@@ -7974,7 +7974,7 @@ static void megasas_detach_one(struct pci_dev *pdev)
struct Scsi_Host *host;
struct megasas_instance *instance;
struct fusion_context *fusion;
- u32 pd_seq_map_sz;
+ size_t pd_seq_map_sz;
instance = pci_get_drvdata(pdev);
@@ -8046,9 +8046,9 @@ static void megasas_detach_one(struct pci_dev *pdev)
if (instance->adapter_type != MFI_SERIES) {
megasas_release_fusion(instance);
- pd_seq_map_sz = sizeof(struct MR_PD_CFG_SEQ_NUM_SYNC) +
- (sizeof(struct MR_PD_CFG_SEQ) *
- MAX_PHYSICAL_DEVICES);
+ pd_seq_map_sz =
+ struct_size((struct MR_PD_CFG_SEQ_NUM_SYNC *)0,
+ seq, MAX_PHYSICAL_DEVICES);
for (i = 0; i < 2 ; i++) {
if (fusion->ld_map[i])
dma_free_coherent(&instance->pdev->dev,
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/6] Replace one-element arrays with flexible-array members
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
` (5 preceding siblings ...)
2022-08-15 21:52 ` [PATCH v3 6/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_PD_CFG_SEQ_NUM_SYNC Gustavo A. R. Silva
@ 2022-08-16 19:22 ` Kees Cook
2022-08-23 3:59 ` Martin K. Petersen
2022-09-01 5:12 ` Martin K. Petersen
8 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2022-08-16 19:22 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel, Kashyap Desai,
Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, linux-hardening
On Mon, Aug 15, 2022 at 04:35:19PM -0500, Gustavo A. R. Silva wrote:
> Hi!
>
> This series aims to replace one-element arrays with flexible-array
> members in drivers/scsi/megaraid/
>
> I followed the below steps in order to verify the changes don't
> significally impact the code (.text) section generated by the compiler,
> for each object file involved:
>
> 1. Prepare the build with the following settings and configurations:
>
> linux$ KBF="KBUILD_BUILD_TIMESTAMP=1970-01-01 KBUILD_BUILD_USER=user
> KBUILD_BUILD_HOST=host KBUILD_BUILD_VERSION=1"
> linux$ make $KBF allyesconfig
> linux$ ./scripts/config -d GCOV_KERNEL -d KCOV -d GCC_PLUGINS \
> -d IKHEADERS -d KASAN -d UBSAN \
> -d DEBUG_INFO_NONE \
> -e DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
> linux$ make $KBF olddefconfig
>
> 2. Build drivers/scsi/megaraid/ with the same settings and configurations
> as in Step 1, and copy the generated object files in directory before/
>
> linux$ make -j128 $KBF drivers/scsi/megaraid/
> linux$ mkdir -p before
> linux$ cp drivers/scsi/megaraid/*.o before/
>
> 3. Implement all the needed changes and create the patch series. In this
> case, six patches.
>
> linux$ vi code.c
> ...do the magic :)
> linux$ git format-patch ...all the rest
>
> 4. Apply a patch at a time (of the previously created series) and, after
> applying EACH patch, build (as in Step 2) drivers/scsi/megaraid/ and
> copy the generated object files in directory after/
>
> 5. Compare the code section (.text) of each before/file.o and
> after/file.o. I use the following bash script:
>
> compare.sh:
> ARGS="--disassemble --demangle --reloc --no-show-raw-insn --section=.text"
> for i in $(cd before && echo *.o); do
> echo $i
> diff -u <(objdump $ARGS before/$i | sed "0,/^Disassembly/d") \
> <(objdump $ARGS after/$i | sed "0,/^Disassembly/d")
> done
>
> linux$ ./compare.sh > code_comparison.diff
>
> 6. Open the code_comparison.diff file from the example above, look for
> any differences that might show up and analyze them in order to
> determine their impact, and what (if something) should be changed
> or study further.
>
> The above process (code section comparison of object files) is based on
> this[0] blog post by Kees Cook. The compiler used to build the code was
> GCC-12.
>
> In this series I only found the following sorts of differences in files
> megaraid_sas.o and megaraid_sas_base.o:
>
> ...
> ...@@ -7094,24 +7094,24 @@
> 6302: movq $0x0,0x1e20(%rbx)
> 630d: test %r15,%r15
> 6310: je 6316 <megasas_aen_polling+0x56>
> - 6312: R_X86_64_PC32 .text.unlikely+0x3ae3
> + 6312: R_X86_64_PC32 .text.unlikely+0x3ae0
> 6316: mov 0x0(%rip),%eax # 631c <megasas_aen_polling+0x5c>
> 6318: R_X86_64_PC32 event_log_level-0x4
> 631c: mov 0xc(%r15),%r14d
> 6320: lea 0x2(%rax),%edx
> 6323: cmp $0x6,%edx
> 6326: ja 632c <megasas_aen_polling+0x6c>
> - 6328: R_X86_64_PC32 .text.unlikely+0x3ac3
> + 6328: R_X86_64_PC32 .text.unlikely+0x3ac0
> 632c: mov %r14d,%edx
> 632f: sar $0x18,%edx
> 6332: mov %edx,%ecx
> 6334: cmp %eax,%edx
> 6336: jge 633c <megasas_aen_polling+0x7c>
> - 6338: R_X86_64_PC32 .text.unlikely+0x399c
> + 6338: R_X86_64_PC32 .text.unlikely+0x3999
> ...
>
> All of them have to do with the relocation of symbols in the
> .text.unlikely subsection and they don't seem to be of any actual
> relevance. So, we can safely ignore them.
If there's a revision of this series, it might make sense to explicitly
state "no binary code differences" for these changes. (The location of
the relocations don't matter, as you say.)
Reviewed-by: Kees Cook <keescook@chromium.org>
> Also, notice there is an open issue in bugzilla.kernel.org [1] that's
> seems could be fixed by this series. :)
>
> 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 [2].
>
> 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://github.com/KSPP/linux/issues/109
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215943 [1]
> Link: https://reviews.llvm.org/D126864 [2]
>
> Thanks
>
> [0] https://outflux.net/blog/archives/2022/06/24/finding-binary-differences/
>
> Changes in v3:
> - Split the struct_size() changes into a couple of separate patches.
> - Use objdump to compare the code (.text) sections of the object
> files before and after the changes.
> - Modify MR_FW_RAID_MAP_ALL and MR_DRV_RAID_MAP_ALL structures. Change
> suggested by Kees Cook.
>
> Changes in v2:
> - Revert changes in struct MR_FW_RAID_MAP_ALL.
>
> Gustavo A. R. Silva (6):
> scsi: megaraid_sas: Replace one-element array with flexible-array
> member in MR_FW_RAID_MAP
> scsi: megaraid_sas: Replace one-element array with flexible-array
> member in MR_FW_RAID_MAP_DYNAMIC
> scsi: megaraid_sas: Replace one-element array with flexible-array
> member in MR_DRV_RAID_MAP
> scsi: megaraid_sas: Replace one-element array with flexible-array
> member in MR_PD_CFG_SEQ_NUM_SYNC
> scsi: megaraid_sas: Use struct_size() in code related to struct
> MR_FW_RAID_MAP
> scsi: megaraid_sas: Use struct_size() in code related to struct
> MR_PD_CFG_SEQ_NUM_SYNC
>
> drivers/scsi/megaraid/megaraid_sas_base.c | 20 ++++++++++----------
> drivers/scsi/megaraid/megaraid_sas_fp.c | 6 +++---
> drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 +-
> drivers/scsi/megaraid/megaraid_sas_fusion.h | 12 ++++++------
> 4 files changed, 20 insertions(+), 20 deletions(-)
>
> --
> 2.34.1
>
--
Kees Cook
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/6] Replace one-element arrays with flexible-array members
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
` (6 preceding siblings ...)
2022-08-16 19:22 ` [PATCH v3 0/6] Replace one-element arrays with flexible-array members Kees Cook
@ 2022-08-23 3:59 ` Martin K. Petersen
2022-08-23 16:55 ` Gustavo A. R. Silva
2022-09-01 5:12 ` Martin K. Petersen
8 siblings, 1 reply; 11+ messages in thread
From: Martin K. Petersen @ 2022-08-23 3:59 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel, Kashyap Desai,
Sumit Saxena, Shivasharan S, James E.J. Bottomley,
Martin K. Petersen, Kees Cook, linux-hardening
Gustavo,
> This series aims to replace one-element arrays with flexible-array
> members in drivers/scsi/megaraid/
Applied to 6.1/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/6] Replace one-element arrays with flexible-array members
2022-08-23 3:59 ` Martin K. Petersen
@ 2022-08-23 16:55 ` Gustavo A. R. Silva
0 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2022-08-23 16:55 UTC (permalink / raw)
To: Martin K. Petersen
Cc: megaraidlinux.pdl, linux-scsi, linux-kernel, Kashyap Desai,
Sumit Saxena, Shivasharan S, James E.J. Bottomley, Kees Cook,
linux-hardening
On Mon, Aug 22, 2022 at 11:59:22PM -0400, Martin K. Petersen wrote:
>
> Gustavo,
>
> > This series aims to replace one-element arrays with flexible-array
> > members in drivers/scsi/megaraid/
>
> Applied to 6.1/scsi-staging, thanks!
Great. :)
Thanks, Martin.
--
Gustavo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/6] Replace one-element arrays with flexible-array members
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
` (7 preceding siblings ...)
2022-08-23 3:59 ` Martin K. Petersen
@ 2022-09-01 5:12 ` Martin K. Petersen
8 siblings, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2022-09-01 5:12 UTC (permalink / raw)
To: Gustavo A. R. Silva, linux-kernel, linux-scsi, megaraidlinux.pdl
Cc: Martin K . Petersen, linux-hardening, James E.J. Bottomley,
Kees Cook, Kashyap Desai, Sumit Saxena, Shivasharan S
On Mon, 15 Aug 2022 16:35:19 -0500, Gustavo A. R. Silva wrote:
> This series aims to replace one-element arrays with flexible-array
> members in drivers/scsi/megaraid/
>
> I followed the below steps in order to verify the changes don't
> significally impact the code (.text) section generated by the compiler,
> for each object file involved:
>
> [...]
Applied to 6.1/scsi-queue, thanks!
[1/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP
https://git.kernel.org/mkp/scsi/c/ac23b92b27e3
[2/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP_DYNAMIC
https://git.kernel.org/mkp/scsi/c/204a29a169f4
[3/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_DRV_RAID_MAP
https://git.kernel.org/mkp/scsi/c/eeb3bab77244
[4/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_PD_CFG_SEQ_NUM_SYNC
https://git.kernel.org/mkp/scsi/c/ee92366a8439
[5/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_FW_RAID_MAP
https://git.kernel.org/mkp/scsi/c/41e830269d68
[6/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_PD_CFG_SEQ_NUM_SYNC
https://git.kernel.org/mkp/scsi/c/48658213202c
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-09-01 5:13 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-15 21:35 [PATCH v3 0/6] Replace one-element arrays with flexible-array members Gustavo A. R. Silva
2022-08-15 21:40 ` [PATCH v3 1/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP Gustavo A. R. Silva
2022-08-15 21:42 ` [PATCH v3 2/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP_DYNAMIC Gustavo A. R. Silva
2022-08-15 21:46 ` [PATCH v3 3/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_DRV_RAID_MAP Gustavo A. R. Silva
2022-08-15 21:49 ` [PATCH v3 4/6] scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_PD_CFG_SEQ_NUM_SYNC Gustavo A. R. Silva
2022-08-15 21:51 ` [PATCH v3 5/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_FW_RAID_MAP Gustavo A. R. Silva
2022-08-15 21:52 ` [PATCH v3 6/6] scsi: megaraid_sas: Use struct_size() in code related to struct MR_PD_CFG_SEQ_NUM_SYNC Gustavo A. R. Silva
2022-08-16 19:22 ` [PATCH v3 0/6] Replace one-element arrays with flexible-array members Kees Cook
2022-08-23 3:59 ` Martin K. Petersen
2022-08-23 16:55 ` Gustavo A. R. Silva
2022-09-01 5:12 ` 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.