New Image format: headers hashes.

- 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)?
- Does the new format assure that the complete image (_all_ sections, images, ramdisks, configurations, blobs) are covered by at least an hash
value? should mkimage add at least a hash value for the root node?

Quoted 12 lines · Luigi 'Comio' Mantellini

- 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)?
- Does the new format assure that the complete image (_all_ sections,
images, ramdisks, configurations, blobs) are covered by at least an hash
value? should mkimage add at least a hash value for the root node?

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? 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.

Any comments?

thanks

luigi

Quoted 5 lines · Luigi 'Comio' Mantellini

- 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)?

Quoted 4 lines

- Does the new format assure that the complete image (_all_ sections, images, ramdisks, configurations, blobs) are covered by at least an hash
value?

Quoted 1 line

should mkimage add at least a hash value for the root node?

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?

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.

Hi Luigi,

Quoted 7 lines · You

Quoted 5 lines · Luigi 'Comio' Mantellini

- 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.

Quoted 4 lines

Quoted 4 lines

- 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.

Quoted 2 lines

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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:

common/image.c +0 -1

index f04826a..850a9e4 100644
@@ -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

common/image.c +0 -0

@@ -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

HTH,
Bartlomiej

Hi Bartlomiej,

thank you for the answers. See my comments below.

Quoted 59 lines · Bartlomiej Sieka

Hi Luigi,

Quoted 4 lines · You

On gio, 2008-03-27 at 12:13 +0100, L
- 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.

Quoted 3 lines

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:

common/image.c +0 -1

index f04826a..850a9e4 100644
@@ -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

common/image.c +0 -0

@@ -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

Quoted 4 lines

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).

Quoted 5 lines

Quoted 1 line

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).

"objcopy -O binary" which is used to create u-boot.bin from u-boot doesn't remove unused functions -- code for fit_image_set_hashes (and other functions removed from under #ifdef USE_HOSTCC by the patch) is present in u-boot.bin.

If you add enough of such dead code, you'll see the increase in size of u-boot.bin, or run into linking problems -- depending on the particular linker script used for the target.

Regards,
Bartlomiej