diff mbox series

uboot-sign: fix U-Boot binary with public key

Message ID c881ce7a-be09-4190-b209-e4de5b55ff4e@gmail.com
State Accepted, archived
Commit 0d14e99aa18ee38293df63d585fafc270a4538be
Headers show
Series uboot-sign: fix U-Boot binary with public key | expand

Commit Message

Clayton Casciato Nov. 22, 2024, 3 p.m. UTC
Fixes [YOCTO #15649]

The U-Boot binary in the "deploy" directory is missing the public key
when the removed logic branch is used.

The simple concatenation of the binary and DTB with public key works as
expected on a BeagleBone Black.

Given:
MACHINE = beaglebone-yocto
UBOOT_SIGN_KEYNAME = "dev"

Post-patch (poky/build/tmp/deploy/images/beaglebone-yocto):
$ hexdump -e "16 \"%_p\" \"\\n\"" u-boot-beaglebone-yocto.dtb \
| tr -d '\n' | grep -o 'key-dev'
key-dev

$ hexdump -e "16 \"%_p\" \"\\n\"" u-boot.img \
| tr -d '\n' | grep -o 'key-dev'
key-dev

Non-Poky BeagleBone Black testing (Scarthgap):
U-Boot 2024.01 [...]
[...]
Using 'conf-ti_omap_am335x-boneblack.dtb' configuration
Verifying Hash Integrity ... sha256,rsa4096:dev+ OK
Trying 'kernel-1' kernel subimage
[...]

Signed-off-by: Clayton Casciato <majortomtosourcecontrol@gmail.com>
---
Sponsor: 21SoftWare LLC

 meta/classes-recipe/uboot-sign.bbclass | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Comments

Ryan Eatmon Dec. 6, 2024, 8:22 p.m. UTC | #1
On 11/22/2024 9:00 AM, Clayton Casciato via lists.openembedded.org wrote:
> Fixes [YOCTO #15649]
> 
> The U-Boot binary in the "deploy" directory is missing the public key
> when the removed logic branch is used.
> 
> The simple concatenation of the binary and DTB with public key works as
> expected on a BeagleBone Black.
> 
> Given:
> MACHINE = beaglebone-yocto
> UBOOT_SIGN_KEYNAME = "dev"
> 
> Post-patch (poky/build/tmp/deploy/images/beaglebone-yocto):
> $ hexdump -e "16 \"%_p\" \"\\n\"" u-boot-beaglebone-yocto.dtb \
> | tr -d '\n' | grep -o 'key-dev'
> key-dev
> 
> $ hexdump -e "16 \"%_p\" \"\\n\"" u-boot.img \
> | tr -d '\n' | grep -o 'key-dev'
> key-dev
> 
> Non-Poky BeagleBone Black testing (Scarthgap):
> U-Boot 2024.01 [...]
> [...]
> Using 'conf-ti_omap_am335x-boneblack.dtb' configuration
> Verifying Hash Integrity ... sha256,rsa4096:dev+ OK
> Trying 'kernel-1' kernel subimage
> [...]
> 
> Signed-off-by: Clayton Casciato <majortomtosourcecontrol@gmail.com>
> ---
> Sponsor: 21SoftWare LLC
> 
>   meta/classes-recipe/uboot-sign.bbclass | 8 +-------
>   1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
> index a17be745ce..7ee73b872a 100644
> --- a/meta/classes-recipe/uboot-sign.bbclass
> +++ b/meta/classes-recipe/uboot-sign.bbclass
> @@ -122,13 +122,7 @@ concat_dtb() {
>   	# If we're not using a signed u-boot fit, concatenate SPL w/o DTB & U-Boot DTB
>   	# with public key (otherwise U-Boot will be packaged by uboot_fitimage_assemble)
>   	if [ "${SPL_SIGN_ENABLE}" != "1" ] ; then
> -		if [ "x${UBOOT_SUFFIX}" = "ximg" -o "x${UBOOT_SUFFIX}" = "xrom" ] && \
> -			[ -e "${UBOOT_DTB_BINARY}" ]; then
> -			oe_runmake EXT_DTB="${UBOOT_DTB_SIGNED}" ${UBOOT_MAKE_TARGET}
> -			if [ -n "${binary}" ]; then
> -				cp ${binary} ${UBOOT_BINARYNAME}-${type}.${UBOOT_SUFFIX}
> -			fi
> -		elif [ -e "${UBOOT_NODTB_BINARY}" -a -e "${UBOOT_DTB_BINARY}" ]; then
> +		if [ -e "${UBOOT_NODTB_BINARY}" -a -e "${UBOOT_DTB_BINARY}" ]; then
>   			if [ -n "${binary}" ]; then
>   				cat ${UBOOT_NODTB_BINARY} ${UBOOT_DTB_SIGNED} | tee ${binary} > \
>   					${UBOOT_BINARYNAME}-${type}.${UBOOT_SUFFIX}


I have been chasing down all week trying to figure out why our master 
builds all stopped booting properly.  I would have responded to this 
patch sooner, but I also got bit by the BB_DANGLINGAPPENDS_WARNONLY that 
tanked our nightly builds around the same time this patch was accepted 
and the Thanskgiving holiday so that I wasn't even looking at the fact 
that our builds were tanked until Monday.

Removing the entire first if clause just to get to the second might fix 
the bug you were looking for, but our code was relying on that first if 
block to work correctly.

The issue is that UBOOT_SUFFIX is saying to create an .img file, but 
removing the first if clause just installs the .bin file under the 
suffix .img which is just wrong.

I think this patch needs to be reverted and a second attempt at fixing 
the bug needs to be made.



> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#207623): https://lists.openembedded.org/g/openembedded-core/message/207623
> Mute This Topic: https://lists.openembedded.org/mt/109723518/6551054
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [reatmon@ti.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
index a17be745ce..7ee73b872a 100644
--- a/meta/classes-recipe/uboot-sign.bbclass
+++ b/meta/classes-recipe/uboot-sign.bbclass
@@ -122,13 +122,7 @@  concat_dtb() {
 	# If we're not using a signed u-boot fit, concatenate SPL w/o DTB & U-Boot DTB
 	# with public key (otherwise U-Boot will be packaged by uboot_fitimage_assemble)
 	if [ "${SPL_SIGN_ENABLE}" != "1" ] ; then
-		if [ "x${UBOOT_SUFFIX}" = "ximg" -o "x${UBOOT_SUFFIX}" = "xrom" ] && \
-			[ -e "${UBOOT_DTB_BINARY}" ]; then
-			oe_runmake EXT_DTB="${UBOOT_DTB_SIGNED}" ${UBOOT_MAKE_TARGET}
-			if [ -n "${binary}" ]; then
-				cp ${binary} ${UBOOT_BINARYNAME}-${type}.${UBOOT_SUFFIX}
-			fi
-		elif [ -e "${UBOOT_NODTB_BINARY}" -a -e "${UBOOT_DTB_BINARY}" ]; then
+		if [ -e "${UBOOT_NODTB_BINARY}" -a -e "${UBOOT_DTB_BINARY}" ]; then
 			if [ -n "${binary}" ]; then
 				cat ${UBOOT_NODTB_BINARY} ${UBOOT_DTB_SIGNED} | tee ${binary} > \
 					${UBOOT_BINARYNAME}-${type}.${UBOOT_SUFFIX}