[U-Boot-Users] New Image format: headers hashes.
Hi Guys,
I ask you fro a comment regarding the new image format.
I'm studying the mkimage code (and image.c file) to understand how new image works. I have the following observations/questions:
- 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?
Thanks a lot,
luigi
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)?
- 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 Industrie Dial Face S.p.A. 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 E-mail: luigi.mantellini@idf-hit.com GPG fingerprint: 3DD1 7B71 FBDF 6376 1B4A B003 175F E979 907E 1650
Continuing my monologue:
An observation about the fit_image_check_hashes and fit_image_set_hashes functions:
- The hashes protect only the data sections of the image nodes... ignoring the others fields (like "load", "entry", etc...). I don't like this :S I think that the hash values must protect all fields of a node (like the "old" image format).
to be continue...
ciao
luigi
On gio, 2008-03-27 at 17:06 +0100, Luigi 'Comio' Mantellini 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)?
- 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
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.
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.
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
HTH, Bartlomiej
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 filename285875 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 'Comio' Mantellini wrote:
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, 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.
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 filename285875 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).
"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
Hi Bartlomej,
On gio, 2008-04-03 at 13:48 +0200, Bartlomiej Sieka wrote: ...
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.
I'm sorry, I believed (wrongly) that linker was sufficiently intelligent to discard the unused symbols and reallocate the rest. Do you know tools (or ld options) that allow to discard unused symbols/functions?
Thanks a lot. today I learned something new.
luigi
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
participants (2)
-
Bartlomiej Sieka -
Luigi 'Comio' Mantellini