Kconfig: enable CONFIG_BUILD_OUTPUT_STRIPPED by default #51737

marc-hb · 2022-10-28T05:30:31Z

zephyr.strip is deterministic / reproducible while zephyr.elf is not because it has absolute filesystem paths. Create zephyr.strip by default.

This helps investigation of reproducibility issues like the one recently fixed by commit f896fc2 ("scripts: gen_handles: Sort the device handles")

The extra disk space and build time are minimal and dwarfed by the security value: having a reproducible binary that can be checksummed makes it trivial to verify a Software Bill of Materials (https://www.cisa.gov/sbom) and help with supply chain attacks.

See also https://reproducible-builds.org/

Signed-off-by: Marc Herbert [email protected]

nordicjm · 2022-11-01T07:31:19Z

Kconfig.zephyr

@@ -543,6 +543,7 @@ endif # BUILD_OUTPUT_UF2

 config BUILD_OUTPUT_STRIPPED
 	bool "Build a stripped binary"
+	default y


This helps investigation of reproducibility issues like the one recently fixed by commit

So why should this be enabled by default for every build when it provides no use to nearly every zephyr user and those that want to use it can enable it manually with menuconfig or in their prj.conf?

agree with @nordicjm .

If someone want to enable this per default, then they can easily do this in a samples prj.conf, in their custom board Kconfig, in their custom Zephyr module, etc.

I'm not convinced this should default to y for everyone.

when it provides no use to nearly every zephyr user and those that want to use

This PR has two "insurance" purposes that you have not yet commented on. It's normal to think you don't need insurance against a risk you have never considered.

Reproducible builds and security, see a sample checksumming use in xtensa-build-zephyr.py: install and checksum .elf, .lst and other files thesofproject/sof#6501. Security is not something users generally perceive as "useful", usually the opposite. This PR is special because it will help with security without anyone even noticing - until they are actually targeted by some supply-chain attack and then realize the value.

Making sure everyone in some "task force" is using the exact same binary when trying to reproduce some very elusive failure. Again this is the kind of feature that you don't know you need until you're in this sort of situation. By that time people have much more urgent things to do than figure out how to tweak everyone's configuration and the configuration of automated builds - with the risk of introducing other changes. With a reproducible build catching issues like f896fc2 can become routine instead and happen long before any "task force" is started.

those that want to use it can enable it manually with menuconfig or in their prj.conf?

That's too late when trying to reproduce binaries already released and/or built by CI. It's also inconvenient when trying to compare binaries across multiple people including validation people or other non-developers who have never configured anything in Zephyr.

Finally let's also have a look at a sample, current, top-level build/zephyr/ directory, see below (I omitted the subdirectories). I can see 3 .elf files and 4 .map files. besides to the build system itself, which of these files are useful and to whom? What about all the other files?

This PR would add this reproducible file for a totally negligible build time and disk space

43572 Nov 1 22:57 zephyr.strip

$ west build -b qemu_x86 samples/hello_world/ ls -lS build/zephyr/ 440334 Nov 1 22:52 zephyr.lst 414364 Nov 1 22:52 zephyr.elf 414160 Nov 1 22:52 zephyr_pre1.elf 413544 Nov 1 22:52 zephyr_pre0.elf 371436 Nov 1 22:52 libzephyr.a 231181 Nov 1 22:52 zephyr_final.map 231181 Nov 1 22:52 zephyr.map 230997 Nov 1 22:52 zephyr_pre1.map 230116 Nov 1 22:52 zephyr_pre0.map 224567 Nov 1 22:52 edt.pickle 21340 Nov 1 22:52 dts.cmake 11288 Nov 1 22:52 linker.cmd 11228 Nov 1 22:52 linker_zephyr_pre0.cmd 11228 Nov 1 22:52 linker_zephyr_pre1.cmd 4618 Nov 1 22:52 linker_zephyr_pre0.cmd.dep 4618 Nov 1 22:52 linker_zephyr_pre1.cmd.dep 4606 Nov 1 22:52 linker.cmd.dep 4423 Nov 1 22:52 zephyr.dts.pre 3558 Nov 1 22:52 zephyr.stat 3368 Nov 1 22:52 zephyr.dts 885 Nov 1 22:52 dev_handles.c 345 Nov 1 22:52 zephyr.dts.d

Your comments do not convince me of anything useful about this being the default. In the 5/6 years I've used zephyr, I've never needed this feature, as for right now I absolutely do not want to use this feature, you are trying to force a default on everyone and you're not able to convince me with the above that any random zephyr user would get any benefit from this at all.

I understand this has no value for you and I'm sorry you don't see the generic problems this helps with. However this is mysterious to me:

I absolutely do not want to use this feature,

It's mysterious because most people would never notice this change. Adding a (reproducible at last) kilobytes-big .elf file in the megabytes-big build/ directory without affecting any other existing file is really not a "feature".

So I understand that you don't see any value but I don't understand why you seem to care so much. What problem(s) does this cause in practice for you?

I do not see any value of enabling this by default as well, this does not give us anything, if you need the stripped binary, you can just enable it, if you want to verify reproducibility and check for reproducibility, you can enable it, but build a strpped version of each app/test by default is not going to guarantee reproducibility, so it is some kind of false advertisement: "zephyr.strip is deterministic / reproducible" (If the binary is not reproducible in the first place, the stripped output will not be....)

It's mysterious because most people would never notice this change. Adding a (reproducible at last) kilobytes-big .elf file in the megabytes-big build/ directory without affecting any other existing file is really not a "feature".

Oh the change would be noticeable, especially for CI given that thousands of tests run, adding a completely pointless additional build stage and output file would start having a time impact on all CIs runs for all PRs.

Oh the change would be noticeable, especially for CI given that thousands of tests run, adding a completely pointless additional build stage and output file would start having a time impact on all CIs runs for all PRs.

This is not what I've measured, what makes you think that?

tejlmand · 2022-11-01T08:20:47Z

CMakeLists.txt

+if(BOARD MATCHES "^native_posix")
+# Using a random toolchain makes zephyr.strip not interesting anyway.
+message("WARNING: not stripping ${BOARD} because of bug #51821")


This is not an acceptable way to do this.

If there is a need for not supporting a feature on a board / arch, then that should be ensured using Kconfig.

But for issue #51821 as fix is ready here: #51837

Fix merged, workaround gone - resolved.

tejlmand · 2022-11-01T08:23:00Z

Kconfig.zephyr

@@ -543,6 +543,7 @@ endif # BUILD_OUTPUT_UF2

 config BUILD_OUTPUT_STRIPPED
 	bool "Build a stripped binary"
+	default y


agree with @nordicjm .

If someone want to enable this per default, then they can easily do this in a samples prj.conf, in their custom board Kconfig, in their custom Zephyr module, etc.

I'm not convinced this should default to y for everyone.

CMakeLists.txt

zephyr.strip is deterministic / reproducible while zephyr.elf is not because it has absolute filesystem paths. Create zephyr.strip by default. This helps investigation of reproducibility issues like the one recently fixed by commit f896fc2 ("scripts: gen_handles: Sort the device handles") The extra disk space and build time are minimal and dwarfed by the security value: having a reproducible binary that can be checksummed makes it trivial to verify a Software Bill of Materials (https://www.cisa.gov/sbom) and help with supply chain attacks. See also https://reproducible-builds.org/ Signed-off-by: Marc Herbert <[email protected]>

outdated

ceolin · 2022-11-02T18:58:55Z

I think we should publish the hash for the stripped binary so other users or even other CIs have something to compare with. I do see value in this to check if some step in the build chain was tampered.

nashif · 2022-11-02T19:20:26Z

Kconfig.zephyr

@@ -543,6 +543,7 @@ endif # BUILD_OUTPUT_UF2

 config BUILD_OUTPUT_STRIPPED
 	bool "Build a stripped binary"
+	default y


I do not see any value of enabling this by default as well, this does not give us anything, if you need the stripped binary, you can just enable it, if you want to verify reproducibility and check for reproducibility, you can enable it, but build a strpped version of each app/test by default is not going to guarantee reproducibility, so it is some kind of false advertisement: "zephyr.strip is deterministic / reproducible" (If the binary is not reproducible in the first place, the stripped output will not be....)

marc-hb

Considering most reviewers so far did not see the purpose I added the checksumming (which was the goal all along), totally rewrote the commit messages and re-submitted #51954. The stripped file is only a means to an end so hopefully the discussion in #51954 can focus on the goals and outputs first and on the (mostly invisible) implementation details like the stripped file only second. If anyone has any suggestion on how to achieve the same goals differently then I'm all ears.

https://en.wikipedia.org/wiki/XY_problem

marc-hb · 2022-11-04T03:26:26Z

Kconfig.zephyr

@@ -543,6 +543,7 @@ endif # BUILD_OUTPUT_UF2

 config BUILD_OUTPUT_STRIPPED
 	bool "Build a stripped binary"
+	default y


Oh the change would be noticeable, especially for CI given that thousands of tests run, adding a completely pointless additional build stage and output file would start having a time impact on all CIs runs for all PRs.

This is not what I've measured, what makes you think that?

marc-hb force-pushed the strip-by-default branch 3 times, most recently from c4fc7ed to 84efedc Compare October 31, 2022 21:51

marc-hb marked this pull request as ready for review October 31, 2022 22:25

marc-hb requested review from tejlmand and nashif as code owners October 31, 2022 22:25

zephyrbot added the area: Build System label Oct 31, 2022

zephyrbot requested a review from jeremybettis October 31, 2022 22:25

zephyrbot assigned tejlmand Oct 31, 2022

marc-hb mentioned this pull request Oct 31, 2022

native_posix: Cmake must be run at least twice to find ${CMAKE_STRIP} #51821

Closed

marc-hb requested review from lgirdwood, kv2019i, abonislawski, aborisovich, aescolar, andyross, ceolin, dbaluta and dcpleung October 31, 2022 22:29

marc-hb mentioned this pull request Oct 31, 2022

xtensa-build-zephyr.py: install and checksum .elf, .lst and other files thesofproject/sof#6501

Merged

nordicjm requested changes Nov 1, 2022

View reviewed changes

tejlmand requested changes Nov 1, 2022

View reviewed changes

aescolar previously requested changes Nov 1, 2022

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

marc-hb marked this pull request as draft November 1, 2022 14:26

marc-hb force-pushed the strip-by-default branch from 84efedc to e7aa947 Compare November 1, 2022 21:51

marc-hb marked this pull request as ready for review November 1, 2022 22:31

marc-hb requested a review from keith-zephyr November 1, 2022 23:12

nashif requested changes Nov 2, 2022

View reviewed changes

marc-hb commented Nov 4, 2022

View reviewed changes

marc-hb closed this Nov 4, 2022

marc-hb mentioned this pull request Nov 4, 2022

cmake: compute and display the reproducible checksum by default #51954

Closed

marc-hb mentioned this pull request Mar 12, 2023

[FEATURE] Make SOF with Zephyr run on i.MX93's A55 core with Jailhouse thesofproject/sof#7192

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kconfig: enable CONFIG_BUILD_OUTPUT_STRIPPED by default #51737

Kconfig: enable CONFIG_BUILD_OUTPUT_STRIPPED by default #51737

marc-hb commented Oct 28, 2022

nordicjm Nov 1, 2022

tejlmand Nov 1, 2022

marc-hb Nov 1, 2022 •

edited

Loading

nordicjm Nov 2, 2022

marc-hb Nov 2, 2022 •

edited

Loading

nashif Nov 2, 2022 •

edited

Loading

nordicjm Nov 3, 2022

marc-hb Nov 4, 2022

tejlmand Nov 1, 2022

marc-hb Nov 1, 2022

tejlmand Nov 1, 2022

ceolin commented Nov 2, 2022

nashif Nov 2, 2022 •

edited

Loading

marc-hb left a comment

marc-hb Nov 4, 2022

Kconfig: enable CONFIG_BUILD_OUTPUT_STRIPPED by default #51737

Kconfig: enable CONFIG_BUILD_OUTPUT_STRIPPED by default #51737

Conversation

marc-hb commented Oct 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb Nov 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb Nov 2, 2022 • edited Loading

Choose a reason for hiding this comment

nashif Nov 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ceolin commented Nov 2, 2022

nashif Nov 2, 2022 • edited Loading

Choose a reason for hiding this comment

marc-hb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb Nov 1, 2022 •

edited

Loading

marc-hb Nov 2, 2022 •

edited

Loading

nashif Nov 2, 2022 •

edited

Loading

nashif Nov 2, 2022 •

edited

Loading