[PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com ---
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count; - uint32_t item_offset_list[]; + uint64_t item_offset_list[]; } __packed;
/* image_capsule_support */
On Fri, Jun 16, 2023 at 10:28:05AM +0200, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
Thank you. Just in case, tools/eficapsule.h has the same struct definition because I didn't want to include U-Boot's corresponding header to avoid including a conflicting header file.
Please fix it as well.
-Takahiro Akashi
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count;
- uint32_t item_offset_list[];
- uint64_t item_offset_list[];
} __packed;
/* image_capsule_support */
2.30.2
Hello Takahir,
Am 17.06.2023 um 02:33 schrieb AKASHI Takahiro:
On Fri, Jun 16, 2023 at 10:28:05AM +0200, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
Thank you. Just in case, tools/eficapsule.h has the same struct definition because I didn't want to include U-Boot's corresponding header to avoid including a conflicting header file.
Please fix it as well.
-Takahiro Akashi
Which header should I fix additionally? In include/efi_api.h the correct type is used, so there is nothing to fix.
Best Regards Malte
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count;
- uint32_t item_offset_list[];
uint64_t item_offset_list[]; } __packed;
/* image_capsule_support */
-- 2.30.2
On Mon, Jun 19, 2023 at 11:45:49AM +0200, Schmidt, Malte wrote:
Hello Takahir,
Am 17.06.2023 um 02:33 schrieb AKASHI Takahiro:
On Fri, Jun 16, 2023 at 10:28:05AM +0200, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
Thank you. Just in case, tools/eficapsule.h has the same struct definition because I didn't want to include U-Boot's corresponding header to avoid including a conflicting header file.
Please fix it as well.
-Takahiro Akashi
Which header should I fix additionally? In include/efi_api.h the correct type is used, so there is nothing to fix.
That is tools/eficapsule.h
-Takahiro Akashi
Best Regards Malte
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count;
- uint32_t item_offset_list[];
- uint64_t item_offset_list[]; } __packed; /* image_capsule_support */
-- 2.30.2
Hello Takahiro,
Am 19.06.2023 um 11:53 schrieb AKASHI Takahiro:
On Mon, Jun 19, 2023 at 11:45:49AM +0200, Schmidt, Malte wrote:
Hello Takahir,
Am 17.06.2023 um 02:33 schrieb AKASHI Takahiro:
On Fri, Jun 16, 2023 at 10:28:05AM +0200, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
Thank you. Just in case, tools/eficapsule.h has the same struct definition because I didn't want to include U-Boot's corresponding header to avoid including a conflicting header file.
Please fix it as well.
-Takahiro Akashi
Which header should I fix additionally? In include/efi_api.h the correct type is used, so there is nothing to fix.
That is tools/eficapsule.h
-Takahiro Akashi
tools/eficapsule is the file to which my patch applies to. Please have another look at it. Otherwise, please explain a bit more detailed what your are suggesting, as I am a bit confused about what your point is.
Best Regards Malte
Best Regards Malte
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count;
- uint32_t item_offset_list[];
- uint64_t item_offset_list[]; } __packed; /* image_capsule_support */
-- 2.30.2
On Mon, Jun 19, 2023 at 12:37:48PM +0200, Schmidt, Malte wrote:
Hello Takahiro,
Am 19.06.2023 um 11:53 schrieb AKASHI Takahiro:
On Mon, Jun 19, 2023 at 11:45:49AM +0200, Schmidt, Malte wrote:
Hello Takahir,
Am 17.06.2023 um 02:33 schrieb AKASHI Takahiro:
On Fri, Jun 16, 2023 at 10:28:05AM +0200, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
Thank you. Just in case, tools/eficapsule.h has the same struct definition because I didn't want to include U-Boot's corresponding header to avoid including a conflicting header file.
Please fix it as well.
-Takahiro Akashi
Which header should I fix additionally? In include/efi_api.h the correct type is used, so there is nothing to fix.
That is tools/eficapsule.h
-Takahiro Akashi
tools/eficapsule is the file to which my patch applies to. Please have another look at it. Otherwise, please explain a bit more detailed what your are suggesting, as I am a bit confused about what your point is.
Right, it's my misunderstanding.
-Takahiro Akashi
Best Regards Malte
Best Regards Malte
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count;
- uint32_t item_offset_list[];
- uint64_t item_offset_list[]; } __packed; /* image_capsule_support */
-- 2.30.2
On 6/16/23 10:28, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count;
- uint32_t item_offset_list[];
- uint64_t item_offset_list[];
Defining the same structure in two places it bad practice. https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
} __packed;
/* image_capsule_support */
On Sun, Jul 09, 2023 at 10:31:55AM +0200, Heinrich Schuchardt wrote:
On 6/16/23 10:28, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count;
- uint32_t item_offset_list[];
- uint64_t item_offset_list[];
Defining the same structure in two places it bad practice. https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11
I had a good reason for adding a tool-specific header, instead of using headers under 'include' dir, when I posted v7 of "efi_loader: capsule: improve capsule authentication support" patch. The cover letter says, === v7 (Nov 16, 2021) ... * define eficapsule.h and include it from mkeficapsule (patch#3) Hopefully, the tool can now compile on non-linux host. ===
If I correctly remember, this reflects the comment below and the succeeding discussions: https://lists.denx.de/pipermail/u-boot/2021-November/465859.html
-Takahiro Akashi
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
} __packed;
/* image_capsule_support */
Am 10. Juli 2023 08:26:37 MESZ schrieb AKASHI Takahiro takahiro.akashi@linaro.org:
On Sun, Jul 09, 2023 at 10:31:55AM +0200, Heinrich Schuchardt wrote:
On 6/16/23 10:28, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt malte.schmidt@weidmueller.com
The data type of item_offset_list shall be UINT64 according to the UEFI [1] specifications.
In include/efi_api.h the correct data type is used. The bug was probably never noticed because of little endianness.
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
tools/eficapsule.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..2099a2e9b8 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header { uint32_t version; uint16_t embedded_driver_count; uint16_t payload_item_count;
- uint32_t item_offset_list[];
- uint64_t item_offset_list[];
Defining the same structure in two places it bad practice. https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11
I had a good reason for adding a tool-specific header, instead of using headers under 'include' dir, when I posted v7 of "efi_loader: capsule: improve capsule authentication support" patch. The cover letter says, === v7 (Nov 16, 2021) ...
- define eficapsule.h and include it from mkeficapsule (patch#3)
Hopefully, the tool can now compile on non-linux host.
If I correctly remember, this reflects the comment below and the succeeding discussions: https://lists.denx.de/pipermail/u-boot/2021-November/465859.html
-Takahiro Akashi
We should be able to factor out a common header file which contains capsule specific structures.
I understand that we have to test building on BSD.
Best regards
Heinrich
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
} __packed;
/* image_capsule_support */
participants (4)
-
AKASHI Takahiro -
Heinrich Schuchardt -
Schmidt, Malte -
Stefan Herbrechtsmeier