From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: megaraidlinux.pdl@broadcom.com, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: Kashyap Desai <kashyap.desai@broadcom.com>,
Sumit Saxena <sumit.saxena@broadcom.com>,
Shivasharan S <shivasharan.srikanteshwara@broadcom.com>,
"James E.J. Bottomley" <jejb@linux.ibm.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
Kees Cook <keescook@chromium.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
linux-hardening@vger.kernel.org
Subject: [PATCH v3 0/6] Replace one-element arrays with flexible-array members
Date: Mon, 15 Aug 2022 16:35:19 -0500 [thread overview]
Message-ID: <cover.1660592640.git.gustavoars@kernel.org> (raw)
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
next reply other threads:[~2022-08-16 1:44 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-15 21:35 Gustavo A. R. Silva [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1660592640.git.gustavoars@kernel.org \
--to=gustavoars@kernel.org \
--cc=jejb@linux.ibm.com \
--cc=kashyap.desai@broadcom.com \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=megaraidlinux.pdl@broadcom.com \
--cc=shivasharan.srikanteshwara@broadcom.com \
--cc=sumit.saxena@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.