* [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type @ 2023-06-16 8:28 Stefan Herbrechtsmeier 2023-06-17 0:33 ` AKASHI Takahiro 2023-07-09 8:31 ` Heinrich Schuchardt 0 siblings, 2 replies; 9+ messages in thread From: Stefan Herbrechtsmeier @ 2023-06-16 8:28 UTC (permalink / raw) To: u-boot Cc: Malte Schmidt, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu 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 */ -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type 2023-06-16 8:28 [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type Stefan Herbrechtsmeier @ 2023-06-17 0:33 ` AKASHI Takahiro 2023-06-19 9:45 ` Schmidt, Malte 2023-07-09 8:31 ` Heinrich Schuchardt 1 sibling, 1 reply; 9+ messages in thread From: AKASHI Takahiro @ 2023-06-17 0:33 UTC (permalink / raw) To: Stefan Herbrechtsmeier Cc: u-boot, Malte Schmidt, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type 2023-06-17 0:33 ` AKASHI Takahiro @ 2023-06-19 9:45 ` Schmidt, Malte 2023-06-19 9:53 ` AKASHI Takahiro 0 siblings, 1 reply; 9+ messages in thread From: Schmidt, Malte @ 2023-06-19 9:45 UTC (permalink / raw) To: AKASHI Takahiro, Stefan Herbrechtsmeier, u-boot, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu 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 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type 2023-06-19 9:45 ` Schmidt, Malte @ 2023-06-19 9:53 ` AKASHI Takahiro 2023-06-19 10:37 ` Schmidt, Malte 0 siblings, 1 reply; 9+ messages in thread From: AKASHI Takahiro @ 2023-06-19 9:53 UTC (permalink / raw) To: Schmidt, Malte Cc: Stefan Herbrechtsmeier, u-boot, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu 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 > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type 2023-06-19 9:53 ` AKASHI Takahiro @ 2023-06-19 10:37 ` Schmidt, Malte 2023-06-19 10:45 ` AKASHI Takahiro 0 siblings, 1 reply; 9+ messages in thread From: Schmidt, Malte @ 2023-06-19 10:37 UTC (permalink / raw) To: AKASHI Takahiro, Stefan Herbrechtsmeier, u-boot, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu 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 >>>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type 2023-06-19 10:37 ` Schmidt, Malte @ 2023-06-19 10:45 ` AKASHI Takahiro 0 siblings, 0 replies; 9+ messages in thread From: AKASHI Takahiro @ 2023-06-19 10:45 UTC (permalink / raw) To: Schmidt, Malte Cc: Stefan Herbrechtsmeier, u-boot, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu 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 > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type 2023-06-16 8:28 [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type Stefan Herbrechtsmeier 2023-06-17 0:33 ` AKASHI Takahiro @ 2023-07-09 8:31 ` Heinrich Schuchardt 2023-07-10 6:26 ` AKASHI Takahiro 1 sibling, 1 reply; 9+ messages in thread From: Heinrich Schuchardt @ 2023-07-09 8:31 UTC (permalink / raw) To: Stefan Herbrechtsmeier Cc: Malte Schmidt, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu, u-boot 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 */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type 2023-07-09 8:31 ` Heinrich Schuchardt @ 2023-07-10 6:26 ` AKASHI Takahiro 2023-07-10 6:41 ` Heinrich Schuchardt 0 siblings, 1 reply; 9+ messages in thread From: AKASHI Takahiro @ 2023-07-10 6:26 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Stefan Herbrechtsmeier, Malte Schmidt, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu, u-boot 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 */ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type 2023-07-10 6:26 ` AKASHI Takahiro @ 2023-07-10 6:41 ` Heinrich Schuchardt 0 siblings, 0 replies; 9+ messages in thread From: Heinrich Schuchardt @ 2023-07-10 6:41 UTC (permalink / raw) To: AKASHI Takahiro Cc: Stefan Herbrechtsmeier, Malte Schmidt, Malte Schmidt, Stefan Herbrechtsmeier, Etienne Carriere, Ilias Apalodimas, Masahisa Kojima, Sughosh Ganu, u-boot 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 */ >> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-07-10 6:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-16 8:28 [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type Stefan Herbrechtsmeier 2023-06-17 0:33 ` AKASHI Takahiro 2023-06-19 9:45 ` Schmidt, Malte 2023-06-19 9:53 ` AKASHI Takahiro 2023-06-19 10:37 ` Schmidt, Malte 2023-06-19 10:45 ` AKASHI Takahiro 2023-07-09 8:31 ` Heinrich Schuchardt 2023-07-10 6:26 ` AKASHI Takahiro 2023-07-10 6:41 ` Heinrich Schuchardt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox