| Message ID | 20240326110656.21110-1-javier.viguera@digi.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Fabio Estevam |
| Headers | show |
| Series | arm: imx: fix signature_block_hdr struct fields order | expand |
Hi Javier, On Tue, Mar 26, 2024 at 8:07 AM Javier Viguera <javier.viguera@digi.com> wrote: > > According to the documentation (for example AN13994), for AHAB-enabled > devices the format of the signature block is: > > +--------------+--------------+--------------+-------------+ > | Tag | Length - msb | Length - lsb | Version | > +--------------+--------------+--------------+-------------+ > | SRK Table offset | Certificate offset | > +-----------------------------+----------------------------+ > | Blob offset | Signature offset | > +-----------------------------+----------------------------+ Could you elaborate more about this and share more details? Have you seen a run-time error or did you catch it by code inspection? Please clarify. Thanks
+Ye > Subject: [PATCH] arm: imx: fix signature_block_hdr struct fields order > > According to the documentation (for example AN13994), for AHAB-enabled > devices the format of the signature block is: > > +--------------+--------------+--------------+-------------+ > | Tag | Length - msb | Length - lsb | Version | > +--------------+--------------+--------------+-------------+ > | SRK Table offset | Certificate offset | > +-----------------------------+----------------------------+ > | Blob offset | Signature offset | > +-----------------------------+----------------------------+ > > Signed-off-by: Javier Viguera <javier.viguera@digi.com> > --- > include/imx_container.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/imx_container.h b/include/imx_container.h index > 54cd684e35d5..691c764b3e5b 100644 > --- a/include/imx_container.h > +++ b/include/imx_container.h > @@ -50,10 +50,10 @@ struct signature_block_hdr { > u8 length_lsb; > u8 length_msb; > u8 tag; > - u16 srk_table_offset; > u16 cert_offset; > - u16 blob_offset; > + u16 srk_table_offset; > u16 signature_offset; > + u16 blob_offset; > u32 reserved; > } __packed; >
Hi Fabio,
First, sorry for the delay. I was out of the office (Easter).
-----Original Message-----
From: Fabio Estevam <festevam@gmail.com>
Could you elaborate more about this and share more details?
Have you seen a run-time error or did you catch it by code inspection?
The struct has the fields in the wrong order according to other documentation from NXP (see for example the AN13994 - Encrypted Boot on AHAB-enabled devices).
In the current upstream u-boot code there is NO runtime error, as those fields are not being used. Only ' get_container_size`on `arch/arm/mach-imx/image-container.c` defines a variable of that type. But then it only uses the "length_lsb" and "length_msb" fiels, which are at the beginning of the struct and thus in the correct position.
On the other hand, we were hit by this problem in some bootloader encryption related code we do in our (as of Digi) BSP. We get the blob offset from the signature block header to read the data encryption key blob and due to the wrong order this patch fixes, we were getting a wrong position of the DEK blob.
Hope this clears it up a bit.
--
Javier Viguera
Software Engineer at Digi International.
Hi Javier, On Thu, Apr 4, 2024 at 8:58 AM Viguera, Javier <Javier.Viguera@digi.com> wrote: > The struct has the fields in the wrong order according to other documentation from NXP (see for example the AN13994 - Encrypted Boot on AHAB-enabled devices). > > In the current upstream u-boot code there is NO runtime error, as those fields are not being used. Only ' get_container_size`on `arch/arm/mach-imx/image-container.c` defines a variable of that type. But then it only uses the "length_lsb" and "length_msb" fiels, which are at the beginning of the struct and thus in the correct position. > > On the other hand, we were hit by this problem in some bootloader encryption related code we do in our (as of Digi) BSP. We get the blob offset from the signature block header to read the data encryption key blob and due to the wrong order this patch fixes, we were getting a wrong position of the DEK blob. > > Hope this clears it up a bit. Thanks for the clarification. Please put all this explanation into the commit log and send a v2. Thanks
diff --git a/include/imx_container.h b/include/imx_container.h index 54cd684e35d5..691c764b3e5b 100644 --- a/include/imx_container.h +++ b/include/imx_container.h @@ -50,10 +50,10 @@ struct signature_block_hdr { u8 length_lsb; u8 length_msb; u8 tag; - u16 srk_table_offset; u16 cert_offset; - u16 blob_offset; + u16 srk_table_offset; u16 signature_offset; + u16 blob_offset; u32 reserved; } __packed;
According to the documentation (for example AN13994), for AHAB-enabled devices the format of the signature block is: +--------------+--------------+--------------+-------------+ | Tag | Length - msb | Length - lsb | Version | +--------------+--------------+--------------+-------------+ | SRK Table offset | Certificate offset | +-----------------------------+----------------------------+ | Blob offset | Signature offset | +-----------------------------+----------------------------+ Signed-off-by: Javier Viguera <javier.viguera@digi.com> --- include/imx_container.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)