On 5/3/23 10:06, Ilias Apalodimas wrote:
On Tue, 2 May 2023 at 16:12, Rui Miguel Silva <rui.si...@linaro.org> wrote:

Hi Rui,

The fwu metadata in the metadata partitions
should/are packed to guarantee that the info is
correct in all platforms. Also the size of them
are used to calculate the crc32 and that is important
to get it right.

Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
  include/fwu_mdata.h | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/fwu_mdata.h b/include/fwu_mdata.h
index 8fda4f4ac225..c61221a91735 100644
--- a/include/fwu_mdata.h
+++ b/include/fwu_mdata.h
@@ -22,7 +22,7 @@ struct fwu_image_bank_info {
         efi_guid_t  image_uuid;
         uint32_t accepted;
         uint32_t reserved;
-};
+} __packed;

This structure is naturally packed. Why should adding a __packed
attribute make a difference?


  /**
   * struct fwu_image_entry - information for a particular type of image
@@ -38,7 +38,7 @@ struct fwu_image_entry {
         efi_guid_t image_type_uuid;
         efi_guid_t location_uuid;
         struct fwu_image_bank_info img_bank_info[CONFIG_FWU_NUM_BANKS];
-};
+} __packed;

ditto


  /**
   * struct fwu_mdata - FWU metadata structure for multi-bank updates
@@ -62,6 +62,6 @@ struct fwu_mdata {
         uint32_t previous_active_index;

         struct fwu_image_entry img_entry[CONFIG_FWU_NUM_IMAGES_PER_BANK];
-};
+} __packed;

Depending on the alignment of efi_guid_t __packed might make a
difference here. But as efi_guid_t is defined as 4 aligned I can't see
an impact here either.

Best regards

Heinrich


  #endif /* _FWU_MDATA_H_ */
--
2.40.0


Yep, that's a good catch.  The spec defines 'just' the offsets and not
the C representation, so those should be indeed packed.

Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>




Reply via email to