Skip to content

Replace one-element arrays with flexible-array members

Gustavo A. R. Silva linux-hardening linux-scsi lkml 3 versions – Applied

Status from this message: Applied to 6.1/scsi-staging, thanks!

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v3 Latest6 patches 4 repliesApplied
  2. v26 patches 3 repliesSuperseded
  3. v16 patches 6 repliesSuperseded

Changes in v3 from the cover letter

- 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.

Patches in v3 Applied

#Subject and filesSizeRepliesApplied
1/6scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAPdrivers/scsi/megaraid/megaraid_sas_base.c drivers/scsi/megaraid/megaraid_sas_fp.c drivers/scsi/megaraid/megaraid_sas_fusion.h+4 -40 replies
2/6scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_FW_RAID_MAP_DYNAMICdrivers/scsi/megaraid/megaraid_sas_fusion.h+1 -10 replies
3/6scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_DRV_RAID_MAPdrivers/scsi/megaraid/megaraid_sas_fusion.h+2 -20 replies
4/6scsi: megaraid_sas: Replace one-element array with flexible-array member in MR_PD_CFG_SEQ_NUM_SYNCdrivers/scsi/megaraid/megaraid_sas_base.c drivers/scsi/megaraid/megaraid_sas_fusion.c drivers/scsi/megaraid/megaraid_sas_fusion.h+4 -40 replies
5/6scsi: megaraid_sas: Use struct_size() in code related to struct MR_FW_RAID_MAPdrivers/scsi/megaraid/megaraid_sas_base.c drivers/scsi/megaraid/megaraid_sas_fp.c+6 -60 replies
6/6scsi: megaraid_sas: Use struct_size() in code related to struct MR_PD_CFG_SEQ_NUM_SYNCdrivers/scsi/megaraid/megaraid_sas_base.c+7 -70 replies

Reviews trailers per patch

None of the 6 patches has a Reviewed-by or Acked-by of their own; the series has one for all of them.

PatchKees Cook
SeriesR-b
1/6Enhanced-by
2/6
3/6Enhanced-by
4/6
5/6
6/6

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v3 thread

Cover letter [PATCH v3 0/6] Replace one-element arrays with flexible-array members

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].

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(-)
Signature · 1 line
2.34.1