[PATCH] firmware: handle firmware files being links correctly

[PATCH] firmware: handle firmware files being links correctly

[Sascha Hauer]

Sometimes firmware files can be links. When rebuilding barebox correctly
rebuilds the firmware when file the link points to is updated, but the
firmware is not rebuilt when the link itself is changed to link to
another file.

Fix this by including the sha256sum directly in the generated assembly
file by using .byte rather than generating a file containing the
binary sha256sum and include that using .incbin. This way the generated
assembly file for sure changes when the firmware file changes and it's
rebuilt when necessary (and not when it's not).

Reproducer:

export ARCH=arm
make imx_v8_defconfig
echo foo > firmware/foo
echo bar > firmware/bar
ln -sf foo firmware/imx8mm-bl31.bin
make

The following should rebuild the barebox image including the updated
firmware, but doesn't:

ln -sf bar firmware/imx8mm-bl31.bin
make

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 firmware/Makefile | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/firmware/Makefile b/firmware/Makefile
index 0c9da2996f..df9f277475 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -57,6 +57,8 @@ FWSTR     = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
 FWNAME_EXISTS = $(if $(wildcard $(FIRMWARE_DIR)/$(FWNAME)),1,0)
 
 filechk_fwbin = { \
+	SHA=$$(sha256sum $(FIRMWARE_DIR)/$(FWNAME) | sed 's/ .*$$//;s/../0x&, /g;s/, $$//') ;\
+	\
 	echo "/* Generated by $(src)/Makefile */"		;\
 	echo "\#include <asm-generic/pointer.h>"		;\
 	echo ".section .note.GNU-stack,\"\",%progbits"		;\
@@ -80,7 +82,7 @@ filechk_fwbin = { \
 	echo "    .p2align ASM_LGPTR"				;\
 	echo ".global _fw_$(FWSTR)_sha_start"			;\
 	echo "_fw_$(FWSTR)_sha_start:"				;\
-	echo "    .incbin \"$(fwobjdir)/$(FWNAME).sha.bin\""	;\
+	echo "    .byte $${SHA}"				;\
 	echo ".global _fw_$(FWSTR)_sha_end"			;\
 	echo "_fw_$(FWSTR)_sha_end:"				;\
 }
@@ -89,20 +91,12 @@ filechk_fwbin_ext = { \
 	$(filechk_fwbin)					;\
 }
 
-$(obj)/%.gen.S: $(obj)/%.sha.bin FORCE
+$(obj)/%.gen.S: FORCE
 	$(call filechk,fwbin,.rodata.$(FWSTR),)
 
-$(obj)/%.extgen.S: $(obj)/%.sha.bin FORCE
+$(obj)/%.extgen.S: FORCE
 	$(call filechk,fwbin_ext,.pblext.$(FWSTR),a)
 
-$(obj)/%.sha.bin: $(obj)/%.sum FORCE
-	$(call if_changed,sha256bin)
-
-$(obj)/%.sum: FORCE
-	$(if $(wildcard $(FIRMWARE_DIR)/$*), $(call if_changed,sha256sum,$(FIRMWARE_DIR)/$*), @touch $@)
-
-clean-files += *.sha.bin *.sum
-
 # This dependency is used if missing firmware should fail the build immediately
 fwdep-required-y = $(FIRMWARE_DIR)/%
 # This dependency expands to nothing if the file doesn't exist. This allows
-- 
2.47.3

Re: [PATCH] firmware: handle firmware files being links correctly

[Sascha Hauer]

On Wed, Sep 17, 2025 at 11:58:34AM +0200, Sascha Hauer wrote:
> Sometimes firmware files can be links. When rebuilding barebox correctly
> rebuilds the firmware when file the link points to is updated, but the
> firmware is not rebuilt when the link itself is changed to link to
> another file.
> 
> Fix this by including the sha256sum directly in the generated assembly
> file by using .byte rather than generating a file containing the
> binary sha256sum and include that using .incbin. This way the generated
> assembly file for sure changes when the firmware file changes and it's
> rebuilt when necessary (and not when it's not).
> 
> Reproducer:
> 
> export ARCH=arm
> make imx_v8_defconfig
> echo foo > firmware/foo
> echo bar > firmware/bar
> ln -sf foo firmware/imx8mm-bl31.bin
> make
> 
> The following should rebuild the barebox image including the updated
> firmware, but doesn't:
> 
> ln -sf bar firmware/imx8mm-bl31.bin
> make

I ran into this several times now when I tested different TF-A versions.
I linked the bl31 directly into the trusted-firmware build tree and
at some point I wanted to test a known good bl31 binary and changed the
link to that file. The result can be confusing and time consuming, so
after this happened once again it was time to fix this.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] firmware: handle firmware files being links correctly

[Ahmad Fatoum]

On 9/17/25 11:58 AM, Sascha Hauer wrote:
> Sometimes firmware files can be links. When rebuilding barebox correctly
> rebuilds the firmware when file the link points to is updated, but the
> firmware is not rebuilt when the link itself is changed to link to
> another file.

This is surprising. I though GNU make looks at the mtime of the destination.

> Fix this by including the sha256sum directly in the generated assembly
> file by using .byte rather than generating a file containing the
> binary sha256sum and include that using .incbin. This way the generated
> assembly file for sure changes when the firmware file changes and it's
> rebuilt when necessary (and not when it's not).

The if_changed in just one of the $(if branches looks a bit strange, so
it's good to drop it IMO..

> 
> Reproducer:
> 
> export ARCH=arm
> make imx_v8_defconfig
> echo foo > firmware/foo
> echo bar > firmware/bar
> ln -sf foo firmware/imx8mm-bl31.bin
> make
> 
> The following should rebuild the barebox image including the updated
> firmware, but doesn't:
> 
> ln -sf bar firmware/imx8mm-bl31.bin
> make
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

But see suggestion below.

> ---
>  firmware/Makefile | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/firmware/Makefile b/firmware/Makefile
> index 0c9da2996f..df9f277475 100644
> --- a/firmware/Makefile
> +++ b/firmware/Makefile
> @@ -57,6 +57,8 @@ FWSTR     = $(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME))))
>  FWNAME_EXISTS = $(if $(wildcard $(FIRMWARE_DIR)/$(FWNAME)),1,0)
>  
>  filechk_fwbin = { \
> +	SHA=$$(sha256sum $(FIRMWARE_DIR)/$(FWNAME) | sed 's/ .*$$//;s/../0x&, /g;s/, $$//') ;\
> +	\
>  	echo "/* Generated by $(src)/Makefile */"		;\
>  	echo "\#include <asm-generic/pointer.h>"		;\
>  	echo ".section .note.GNU-stack,\"\",%progbits"		;\
> @@ -80,7 +82,7 @@ filechk_fwbin = { \
>  	echo "    .p2align ASM_LGPTR"				;\
>  	echo ".global _fw_$(FWSTR)_sha_start"			;\
>  	echo "_fw_$(FWSTR)_sha_start:"				;\
> -	echo "    .incbin \"$(fwobjdir)/$(FWNAME).sha.bin\""	;\
> +	echo "    .byte $${SHA}"				;\
>  	echo ".global _fw_$(FWSTR)_sha_end"			;\

How about we add here an

.if _fw_$(FWSTR)_sha_start - _fw_$(FWSTR)_sha_end
	.err
	.warnig "sha256sum empty"
.endif

As .byte doesn't require an argument, this would ensure breakage to be
noticed at compile time.

Cheers,
Ahmad

>  }
> @@ -89,20 +91,12 @@ filechk_fwbin_ext = { \
>  	$(filechk_fwbin)					;\
>  }
>  
> -$(obj)/%.gen.S: $(obj)/%.sha.bin FORCE
> +$(obj)/%.gen.S: FORCE
>  	$(call filechk,fwbin,.rodata.$(FWSTR),)
>  
> -$(obj)/%.extgen.S: $(obj)/%.sha.bin FORCE
> +$(obj)/%.extgen.S: FORCE
>  	$(call filechk,fwbin_ext,.pblext.$(FWSTR),a)
>  
> -$(obj)/%.sha.bin: $(obj)/%.sum FORCE
> -	$(call if_changed,sha256bin)
> -
> -$(obj)/%.sum: FORCE
> -	$(if $(wildcard $(FIRMWARE_DIR)/$*), $(call if_changed,sha256sum,$(FIRMWARE_DIR)/$*), @touch $@)
> -
> -clean-files += *.sha.bin *.sum
> -
>  # This dependency is used if missing firmware should fail the build immediately
>  fwdep-required-y = $(FIRMWARE_DIR)/%
>  # This dependency expands to nothing if the file doesn't exist. This allows

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH] firmware: handle firmware files being links correctly

[Sascha Hauer]

On Wed, Sep 17, 2025 at 12:21:57PM +0200, Ahmad Fatoum wrote:
> On 9/17/25 11:58 AM, Sascha Hauer wrote:
> > Sometimes firmware files can be links. When rebuilding barebox correctly
> > rebuilds the firmware when file the link points to is updated, but the
> > firmware is not rebuilt when the link itself is changed to link to
> > another file.
> 
> This is surprising. I though GNU make looks at the mtime of the destination.

It does, but in this case it should also consider the timestamp of the
link itself, not the destination.

> > @@ -80,7 +82,7 @@ filechk_fwbin = { \
> >  	echo "    .p2align ASM_LGPTR"				;\
> >  	echo ".global _fw_$(FWSTR)_sha_start"			;\
> >  	echo "_fw_$(FWSTR)_sha_start:"				;\
> > -	echo "    .incbin \"$(fwobjdir)/$(FWNAME).sha.bin\""	;\
> > +	echo "    .byte $${SHA}"				;\
> >  	echo ".global _fw_$(FWSTR)_sha_end"			;\
> 
> How about we add here an
> 
> .if _fw_$(FWSTR)_sha_start - _fw_$(FWSTR)_sha_end
> 	.err
> 	.warnig "sha256sum empty"
> .endif
> 
> As .byte doesn't require an argument, this would ensure breakage to be
> noticed at compile time.

Ok. The .if expression is reversed. Will change to:

	.if _fw_$(FWSTR)_sha_start + 32 - _fw_$(FWSTR)_sha_end
	.error "sha256sum invalid"
	.endif"

This not only checks for an empty sha sum but also for the correct
length.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] firmware: handle firmware files being links correctly

[Sascha Hauer]

On Wed, 17 Sep 2025 11:58:34 +0200, Sascha Hauer wrote:
> Sometimes firmware files can be links. When rebuilding barebox correctly
> rebuilds the firmware when file the link points to is updated, but the
> firmware is not rebuilt when the link itself is changed to link to
> another file.
> 
> Fix this by including the sha256sum directly in the generated assembly
> file by using .byte rather than generating a file containing the
> binary sha256sum and include that using .incbin. This way the generated
> assembly file for sure changes when the firmware file changes and it's
> rebuilt when necessary (and not when it's not).
> 
> [...]

Applied, thanks!

[1/1] firmware: handle firmware files being links correctly
      https://git.pengutronix.de/cgit/barebox/commit/?id=b10351d3080a (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>