mcuboot: force .hex and .bin same content

[JordanYates]

Ensure that the automatically generated zephyr.signed.hex and zephyr.signed.bin files (and derivatives) have the same logical content. Previously the two files would have different image signatures due to the indepenent calls to the image signing tool.

Note this only resolves the issue when the images are automatically signed, calling west sign manually with still result in different content. Resolving this is much harder as the tool has no knowledge of the commands requires to convert a .hex to a .bin.

Fixes #52271.

[nordicjm]

Quite honestly I don't think the majority of people care about this, some yes but the minority.

[nordicjm]

This is not a transferable way of doing this and requires use of this specific file to achieve this (and this file can now be completely replaced out of tree), would rather something generic is added to the cmake code that can convert any hex to bin (or just use objcopy) and can be called as is needed

[tejlmand]

would rather something generic is added to the cmake code that can convert any hex to bin (or just use objcopy) and can be called as is needed

@nordicjm not sure exactly what you mean by this ?
The use of the elfconvert_command target property from bintools is the way toolchains can specify how files can be converted, and is used also by Zephyr in many places, for example here and elsewhere in that file:

zephyr/CMakeLists.txt

Line 1175 in ccd88a4

COMMAND $<TARGET_PROPERTY:bintools,elfconvert_command>

if you are requesting a common CMake function to the this, but how the current function is implemented is acceptable, then I have mis-read your comment, and I apologize the noise ?

or just use objcopy

not all toolchains support objcopy, which is why toolchains should setup their own elfconvert_command property, which is objcopy for gnu.
Ref:

zephyr/cmake/bintools/gnu/target_bintools.cmake

Lines 23 to 25 in ccd88a4

	# elfconvert to use for transforming an elf file into another format,
	# such as intel hex, s-rec, binary, etc.
	set_property(TARGET bintools PROPERTY elfconvert_command ${CMAKE_OBJCOPY})

zephyr/cmake/bintools/arcmwdt/target_bintools.cmake

Lines 12 to 15 in ccd88a4

	# MWDT toolchain does not support all options in a single command
	# Therefore a CMake script is used, so that multiple commands can be executed
	# successively.
	set_property(TARGET bintools PROPERTY elfconvert_command ${CMAKE_COMMAND})

[tejlmand]

I propose to first sign the hex file, and if both hex and bin output is requested then convert the hex file after signing.

If only bin output is requested, then sign the bin output.

[tejlmand]

instead of introducing yet another Kconfig setting, then why no swap the code here:

zephyr/cmake/mcuboot.cmake

Line 110 in ccd88a4

if(CONFIG_BUILD_OUTPUT_BIN)

and here:

zephyr/cmake/mcuboot.cmake

Line 127 in ccd88a4

if(CONFIG_BUILD_OUTPUT_HEX)

so that order becomes:

  if(CONFIG_BUILD_OUTPUT_HEX)
    # Do signing of hex file
  endif()

  if(CONFIG_BUILD_OUTPUT_BIN AND CONFIG_BUILD_OUTPUT_HEX)
    # Both hex and bin output is enabled, convert signed hex to signed bin to mitigate #52271.
  elseif(CONFIG_BUILD_OUTPUT_BIN)
    # Only bin output is enabled, keep existing image bin signing.
  endif()

[JordanYates]

I went with this approach as west sign current automatically discovers files to be signed, so even without the bin options it still actually signs the .bin. If this file is generated anyway the easiest option IMO is just to overwrite it. Obviously this can be updated but is a larger change.

zephyr/scripts/west_commands/sign.py

Lines 155 to 175 in fd94ca7

	# Decide on output formats.
	formats = []
	bin_exists = build_conf.getboolean('CONFIG_BUILD_OUTPUT_BIN')
	if args.gen_bin:
	self.check_force(bin_exists,
	'--bin given but CONFIG_BUILD_OUTPUT_BIN not set '
	"in build directory's ({}) .config".
	format(build_dir))
	formats.append('bin')
	elif args.gen_bin is None and bin_exists:
	formats.append('bin')
	hex_exists = build_conf.getboolean('CONFIG_BUILD_OUTPUT_HEX')
	if args.gen_hex:
	self.check_force(hex_exists,
	'--hex given but CONFIG_BUILD_OUTPUT_HEX not set '
	"in build directory's ({}) .config".
	format(build_dir))
	formats.append('hex')
	elif args.gen_hex is None and hex_exists:
	formats.append('hex')

I didn't go with if(CONFIG_BUILD_OUTPUT_BIN AND CONFIG_BUILD_OUTPUT_HEX) directly in the cmake just to provide the option to disable this behaviour if it happens to cause issues for someone.

[tejlmand]

I went with this approach as west sign current automatically discovers files to be signed

Right, I had forgotten about the pure west sign behavior.
Having different behavior on west sign and during build is not good.

I didn't go with if(CONFIG_BUILD_OUTPUT_BIN AND CONFIG_BUILD_OUTPUT_HEX) directly in the cmake just to provide the option to disable this behaviour if it happens to cause issues for someone.

and that's a fair point.
But I dislike having an extra Kconfig for this.
and even though it's configurable, there is still a mismatch to how it works when west sign is invoked.

[nordicjm]

@nordicjm not sure exactly what you mean by this ?
The use of the elfconvert_command target property from bintools is the way toolchains can specify how files can be converted, and is used also by Zephyr in many places, for example here and elsewhere in that file:
if you are requesting a common CMake function to the this, but how the current function is implemented is acceptable, then I have mis-read your comment, and I apologize the noise ?

Yes, doing it somewhere else in a common file so that it can be called from here or anywhere as a generic "copy hex to bin" function

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.