[U-Boot-Users] New Image format: headers hashes.

[Luigi 'Comio' Mantellini]

Hi Bartlomiej,

thank you for the answers. See my comments below.

On gio, 2008-04-03 at 11:31 +0200, Bartlomiej Sieka wrote:
> Hi Luigi,
> 
> You wrote:
> > On gio, 2008-03-27 at 12:13 +0100, Luigi 'Comio' Mantellini wrote:
> > 
> >>
> >> - The mkimage calculate hashes before that timestamp is added to
> >> image.
> >> is it better exchange the order of these operations (before add
> >> timestamp then calculate hashes)?
> 
> Please see below.
> 
> 
> >> - Does the new format assure that the complete image (_all_ sections,
> >> images, ramdisks, configurations, blobs) are covered by at least an
> >> hash
> >> value?
> 
> Currently, the hash calculation is performed only over binary data of
> component images (kernel, ramdisk, fdt, etc). In particular, timestamp
> information is not protected by the hash, so the ordering of hash
> calculation and timestamp addition doesn't matter.
> 

Ok. I agree

> 
>  >> should mkimage add at least a hash value for the root node?
> 
> Adding a hash value for the root node only will not be easy. An easier
> approach I think would be to add a hash value for the entire FIT image.
> 
Ok.

> 
> > In addition to the previous question:
> > 
> > - I'm interested to calculate and store from U-boot code but the 
> > fit_image_set_hashes is compiled only when the USE_HOSTCC symbol is 
> > defined. Why? Is there a particular reason?
> 
> There's no need for the function that you mention in current U-Boot
> code, that's the reason why it's under USE_HOSTCC. If you're planning on
> adding code to U-Boot that will use fit_image_set_hashes(), then just
> remove the USE_HOSTCC #define's from around this function.
> 
> 
>  > I think that a lot of
>  > useful functions (ifdef-ed by USE_HOSTCC) should be available also in
>  > u-boot code, anyway the linker will drop the unused functions.
> 
> I don't think the linker will do this in case of U-Boot:
> 
> With the following patch:
> 
> diff --git a/common/image.c b/common/image.c
> index f04826a..850a9e4 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -1950,7 +1950,6 @@ static int calculate_hash (const void *data, int 
> data_len,
>   	return 0;
>   }
> 
> -#ifdef USE_HOSTCC
>   /**
>    * fit_set_hashes - process FIT component image nodes and calculate hashes
>    * @fit: pointer to the FIT format image header
> @@ -2119,7 +2118,6 @@ int fit_image_hash_set_value (void *fit, int 
> noffset, uint
> 
>   	return 0;
>   }
> -#endif /* USE_HOSTCC */
> 
>   /**
>    * fit_image_check_hashes - verify data intergity
> 
> 
> I see:
> 
> $ ${CROSS_COMPILE}nm -S common/image.o | grep fit_image_set_hashes
> 0000155c 000001f4 T fit_image_set_hashes
> $ ${CROSS_COMPILE}nm -S common/libcommon.a | grep fit_image_set_hashes
> 0000155c 000001f4 T fit_image_set_hashes
> $ ${CROSS_COMPILE}nm -S u-boot | grep fit_image_set_hashes
> fffb1924 000001f4 T fit_image_set_hashes
> 
> 
> There's also size increase with the patch applied:
>     text    data     bss     dec     hex filename
>   284615   15556  357120  657291   a078b ./u-boot
> 
>     text    data     bss     dec     hex filename
>   285875   15572  357120  658567   a0c87 u-boot
> 
> 

Before to make this test, you should strip the elf-file in order to omit
the non-used code. Alternatively you should check the size of the
u-boot.bin file (that is striped by definition).

Best regards.

luigi

> HTH,
> Bartlomiej
-- 
     ______       Luigi Mantellini
   .'______'.     R&D - Software
  (.'      '.)    Industrie Dial Face S.p.A.
  ( :=----=: )    Via Canzo, 4
  ('.______.')    20068 Peschiera Borromeo (MI), Italy
   '.______.'     Tel.: +39 02 5167 2813
                  Fax:  +39 02 5167 2459
Ind.  Dial Face   Email: luigi.mantellini at idf-hit.com
www.idf-hit.com   GPG fingerprint: 3DD1 7B71 FBDF 6376 1B4A
                                   B003 175F E979 907E 1650