[PATCH 7/7] tools: kwboot: Add knowledge of Marvell's TIM

Pali Rohár pali at kernel.org
Fri Sep 16 10:11:58 CEST 2022


Hello! I think it does not make sense to hack kwboot to skip validation
of kwbimage format when ad-hoc TIM header is detected. kwboot has now
lot of features which requires and expects valid kwbimage format and is
now written to work specially with 32-bit mvebu ARM BootROMs.

TIM and kwbimage are totally different formats and it really does not
make sense to starting rewriting kwboot to support also other format.
Instead it would be better to write other dedicated tool for it.

For example, there is already tool mox-imager [1], which despite its
name supports all A3720 BootROMS and mvebu64boot [2] which supports
A70x0, A80x0 and CN9130 BootROMS.

[1] - https://gitlab.nic.cz/turris/mox-imager
[2] - https://github.com/pali/mvebu64boot

On Friday 16 September 2022 16:54:23 Chris Packham wrote:
> Marvell's proprietary TIM (trusted image) is used on the Armada-3700 and
> AlledCat5/5X (and possibly others). It has a whole host of features that
> work to implement a version of secure boot.
> 
> Kwboot's interest in this format is simply to detect that the image is
> one of these and not attempt to patch it (the images will work over UART
> boot as-is). This is done by checking for a specific magic value
> ("TIMH") in the first 32bits of the image.
> 
> Signed-off-by: Chris Packham <judge.packham at gmail.com>
> ---
> It might be possible to make the check more robust by validating more of
> the image. There is a checksum field that might be useful for this
> purpose. I haven't done this as I couldn't figure out Marvell's
> validation of this field.
> 
>  tools/kwbimage.h | 29 +++++++++++++++++++++++++++++
>  tools/kwboot.c   |  3 +++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/tools/kwbimage.h b/tools/kwbimage.h
> index 505522332b..8aab26952a 100644
> --- a/tools/kwbimage.h
> +++ b/tools/kwbimage.h
> @@ -224,6 +224,28 @@ struct register_set_hdr_v1 {
>  #define OPT_HDR_V1_BINARY_TYPE   0x2
>  #define OPT_HDR_V1_REGISTER_TYPE 0x3
>  
> +/* TIM (trusted image), Armada 3700, AlleyCat5 */
> +struct tim_block_hdr {
> +	uint32_t signature_id;
> +	uint16_t opcode;
> +	uint16_t blocksize;
> +} __packed;
> +
> +struct tim_hdr {
> +	struct tim_block_hdr hdr;
> +	uint32_t trusted;
> +	uint32_t signed_tim_size;
> +	uint32_t unsigned_tim_size;
> +	uint32_t unique_id;
> +	uint64_t loadaddr;
> +	uint32_t flags;
> +	uint32_t software_prot_version;
> +	uint32_t num_blocks;
> +	uint32_t checksum;
> +} __packed;
> +
> +#define TIM_HDR_SIGNATURE	0x54494d48 /* "TIMH" */
> +
>  /*
>   * Byte 8 of the image header contains the version number. In the v0
>   * header, byte 8 was reserved, and always set to 0. In the v1 header,
> @@ -270,6 +292,13 @@ static inline size_t kwbheader_size_for_csum(const void *header)
>  		return kwbheader_size(header);
>  }
>  
> +static inline bool kwbimage_is_tim(void *img)
> +{
> +	const struct tim_hdr *hdr = img;
> +
> +	return le32_to_cpu(hdr->hdr.signature_id) == TIM_HDR_SIGNATURE;
> +}
> +
>  static inline struct ext_hdr_v0 *ext_hdr_v0_first(void *img)
>  {
>  	struct main_hdr_v0 *mhdr;
> diff --git a/tools/kwboot.c b/tools/kwboot.c
> index da4fe32da2..a9b3d0fd04 100644
> --- a/tools/kwboot.c
> +++ b/tools/kwboot.c
> @@ -1869,6 +1869,9 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
>  	if (*size < sizeof(struct main_hdr_v1))
>  		goto err;
>  
> +	if (kwbimage_is_tim(img))
> +		return 0;
> +
>  	image_ver = kwbimage_version(img);
>  	if (image_ver != 0 && image_ver != 1) {
>  		fprintf(stderr, "Invalid image header version\n");
> -- 
> 2.37.3
> 


More information about the U-Boot mailing list