* [edk2-devel] [PATCH] UefiPayloadPkg: CbParseLib: Fix integer overflow @ 2024-01-08 6:59 Patrick Rudolph 2024-01-12 11:33 ` Sheng Lean Tan 0 siblings, 1 reply; 4+ messages in thread From: Patrick Rudolph @ 2024-01-08 6:59 UTC (permalink / raw) To: devel; +Cc: sean, gua.guo, james.lu, ray.ni, guo.dong, sheng.tan, Patrick Rudolph The IMD entry uses the 32bit start field as relative offset to root. On Ia32X64 this works fine as UINTN is also 32 bit and negative relative offsets are properly calculated due to an integer overflow. On X64 this doesn't work as UINTN is 64 bit and the offset is no longer subtracted, but it's added to the root. Fix that by sign extending the start field to 64 bit. Test: Booting UefiPayloadPkg still works on Ia32X64 and now also works on X64. Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> --- UefiPayloadPkg/Library/CbParseLib/CbParseLib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c index 8a353f77f6..9e149532a7 100644 --- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c +++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c @@ -282,7 +282,7 @@ FindCbMemTable ( for (Idx = 0; Idx < Root->num_entries; Idx++) { if (Entries[Idx].id == TableId) { if (IsImdEntry) { - *MemTable = (VOID *)((UINTN)Entries[Idx].start + (UINTN)Root); + *MemTable = (VOID *)((INTN)(INT32)Entries[Idx].start + (UINTN)Root); } else { *MemTable = (VOID *)(UINTN)Entries[Idx].start; } -- 2.43.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113361): https://edk2.groups.io/g/devel/message/113361 Mute This Topic: https://groups.io/mt/103593206/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] UefiPayloadPkg: CbParseLib: Fix integer overflow 2024-01-08 6:59 [edk2-devel] [PATCH] UefiPayloadPkg: CbParseLib: Fix integer overflow Patrick Rudolph @ 2024-01-12 11:33 ` Sheng Lean Tan 2024-01-12 11:42 ` Guo, Gua 0 siblings, 1 reply; 4+ messages in thread From: Sheng Lean Tan @ 2024-01-12 11:33 UTC (permalink / raw) To: Patrick Rudolph; +Cc: devel, sean, gua.guo, james.lu, ray.ni, guo.dong [-- Attachment #1: Type: text/plain, Size: 2332 bytes --] Hi Gua or Sean, Would you mind to help review this? Thanks! Best Regards, *Lean Sheng Tan* 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany Email: sheng.tan@9elements.com Phone: *+49 234 68 94 188 <+492346894188>* Mobile: *+49 176 76 113842 <+4917676113842>* Registered office: Bochum Commercial register: Amtsgericht Bochum, HRB 17519 Management: Sebastian German, Eray Bazaar Data protection information according to Art. 13 GDPR <https://9elements.com/privacy> On Mon, 8 Jan 2024 at 08:00, Patrick Rudolph <patrick.rudolph@9elements.com> wrote: > The IMD entry uses the 32bit start field as relative offset > to root. On Ia32X64 this works fine as UINTN is also 32 bit and > negative relative offsets are properly calculated due to an > integer overflow. > > On X64 this doesn't work as UINTN is 64 bit and the offset > is no longer subtracted, but it's added to the root. Fix that > by sign extending the start field to 64 bit. > > Test: Booting UefiPayloadPkg still works on Ia32X64 and now also > works on X64. > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > --- > UefiPayloadPkg/Library/CbParseLib/CbParseLib.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c > b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c > index 8a353f77f6..9e149532a7 100644 > --- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c > +++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c > @@ -282,7 +282,7 @@ FindCbMemTable ( > for (Idx = 0; Idx < Root->num_entries; Idx++) { > if (Entries[Idx].id == TableId) { > if (IsImdEntry) { > - *MemTable = (VOID *)((UINTN)Entries[Idx].start + (UINTN)Root); > + *MemTable = (VOID *)((INTN)(INT32)Entries[Idx].start + > (UINTN)Root); > } else { > *MemTable = (VOID *)(UINTN)Entries[Idx].start; > } > -- > 2.43.0 > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113713): https://edk2.groups.io/g/devel/message/113713 Mute This Topic: https://groups.io/mt/103593206/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- [-- Attachment #2: Type: text/html, Size: 5211 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] UefiPayloadPkg: CbParseLib: Fix integer overflow 2024-01-12 11:33 ` Sheng Lean Tan @ 2024-01-12 11:42 ` Guo, Gua 2024-01-15 9:11 ` Sean Rhodes 0 siblings, 1 reply; 4+ messages in thread From: Guo, Gua @ 2024-01-12 11:42 UTC (permalink / raw) To: Tan, Lean Sheng, Rudolph, Patrick Cc: devel@edk2.groups.io, Rhodes, Sean, Lu, James, Ni, Ray, Dong, Guo [-- Attachment #1: Type: text/plain, Size: 2915 bytes --] Reviewed-by: Gua Guo <gua.guo@intel.com> ________________________________ From: Lean Sheng Tan <sheng.tan@9elements.com> Sent: Friday, January 12, 2024 7:33:00 PM To: Rudolph, Patrick <patrick.rudolph@9elements.com> Cc: devel@edk2.groups.io <devel@edk2.groups.io>; Rhodes, Sean <sean@starlabs.systems>; Guo, Gua <gua.guo@intel.com>; Lu, James <james.lu@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong, Guo <guo.dong@intel.com> Subject: Re: [PATCH] UefiPayloadPkg: CbParseLib: Fix integer overflow Hi Gua or Sean, Would you mind to help review this? Thanks! Best Regards, Lean Sheng Tan [http://static.9elements.com/logo-signature.png] 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany Email: sheng.tan@9elements.com<mailto:sheng.tan@9elements.com> Phone: +49 234 68 94 188<tel:+492346894188> Mobile: +49 176 76 113842<tel:+4917676113842> Registered office: Bochum Commercial register: Amtsgericht Bochum, HRB 17519 Management: Sebastian German, Eray Bazaar Data protection information according to Art. 13 GDPR<https://9elements.com/privacy> On Mon, 8 Jan 2024 at 08:00, Patrick Rudolph <patrick.rudolph@9elements.com<mailto:patrick.rudolph@9elements.com>> wrote: The IMD entry uses the 32bit start field as relative offset to root. On Ia32X64 this works fine as UINTN is also 32 bit and negative relative offsets are properly calculated due to an integer overflow. On X64 this doesn't work as UINTN is 64 bit and the offset is no longer subtracted, but it's added to the root. Fix that by sign extending the start field to 64 bit. Test: Booting UefiPayloadPkg still works on Ia32X64 and now also works on X64. Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com<mailto:patrick.rudolph@9elements.com>> --- UefiPayloadPkg/Library/CbParseLib/CbParseLib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c index 8a353f77f6..9e149532a7 100644 --- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c +++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c @@ -282,7 +282,7 @@ FindCbMemTable ( for (Idx = 0; Idx < Root->num_entries; Idx++) { if (Entries[Idx].id == TableId) { if (IsImdEntry) { - *MemTable = (VOID *)((UINTN)Entries[Idx].start + (UINTN)Root); + *MemTable = (VOID *)((INTN)(INT32)Entries[Idx].start + (UINTN)Root); } else { *MemTable = (VOID *)(UINTN)Entries[Idx].start; } -- 2.43.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113720): https://edk2.groups.io/g/devel/message/113720 Mute This Topic: https://groups.io/mt/103593206/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- [-- Attachment #2: Type: text/html, Size: 6411 bytes --] ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] UefiPayloadPkg: CbParseLib: Fix integer overflow 2024-01-12 11:42 ` Guo, Gua @ 2024-01-15 9:11 ` Sean Rhodes 0 siblings, 0 replies; 4+ messages in thread From: Sean Rhodes @ 2024-01-15 9:11 UTC (permalink / raw) To: Guo, Gua Cc: Tan, Lean Sheng, Rudolph, Patrick, devel@edk2.groups.io, Lu, James, Ni, Ray, Dong, Guo [-- Attachment #1: Type: text/plain, Size: 3032 bytes --] Reviewed-by: Sean Rhodes <sean@starlabs.systems> On Fri, 12 Jan 2024 at 11:43, Guo, Gua <gua.guo@intel.com> wrote: > Reviewed-by: Gua Guo <gua.guo@intel.com> > ------------------------------ > *From:* Lean Sheng Tan <sheng.tan@9elements.com> > *Sent:* Friday, January 12, 2024 7:33:00 PM > *To:* Rudolph, Patrick <patrick.rudolph@9elements.com> > *Cc:* devel@edk2.groups.io <devel@edk2.groups.io>; Rhodes, Sean > <sean@starlabs.systems>; Guo, Gua <gua.guo@intel.com>; Lu, James < > james.lu@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong, Guo < > guo.dong@intel.com> > *Subject:* Re: [PATCH] UefiPayloadPkg: CbParseLib: Fix integer overflow > > Hi Gua or Sean, > Would you mind to help review this? > Thanks! > > Best Regards, > *Lean Sheng Tan* > > > > 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany > Email: sheng.tan@9elements.com > Phone: *+49 234 68 94 188 <+492346894188>* > Mobile: *+49 176 76 113842 <+4917676113842>* > > Registered office: Bochum > Commercial register: Amtsgericht Bochum, HRB 17519 > Management: Sebastian German, Eray Bazaar > > Data protection information according to Art. 13 GDPR > <https://9elements.com/privacy> > > > On Mon, 8 Jan 2024 at 08:00, Patrick Rudolph < > patrick.rudolph@9elements.com> wrote: > > The IMD entry uses the 32bit start field as relative offset > to root. On Ia32X64 this works fine as UINTN is also 32 bit and > negative relative offsets are properly calculated due to an > integer overflow. > > On X64 this doesn't work as UINTN is 64 bit and the offset > is no longer subtracted, but it's added to the root. Fix that > by sign extending the start field to 64 bit. > > Test: Booting UefiPayloadPkg still works on Ia32X64 and now also > works on X64. > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > --- > UefiPayloadPkg/Library/CbParseLib/CbParseLib.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c > b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c > index 8a353f77f6..9e149532a7 100644 > --- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c > +++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c > @@ -282,7 +282,7 @@ FindCbMemTable ( > for (Idx = 0; Idx < Root->num_entries; Idx++) { > if (Entries[Idx].id == TableId) { > if (IsImdEntry) { > - *MemTable = (VOID *)((UINTN)Entries[Idx].start + (UINTN)Root); > + *MemTable = (VOID *)((INTN)(INT32)Entries[Idx].start + > (UINTN)Root); > } else { > *MemTable = (VOID *)(UINTN)Entries[Idx].start; > } > -- > 2.43.0 > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113812): https://edk2.groups.io/g/devel/message/113812 Mute This Topic: https://groups.io/mt/103593206/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- [-- Attachment #2: Type: text/html, Size: 7043 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-01-16 6:58 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-08 6:59 [edk2-devel] [PATCH] UefiPayloadPkg: CbParseLib: Fix integer overflow Patrick Rudolph 2024-01-12 11:33 ` Sheng Lean Tan 2024-01-12 11:42 ` Guo, Gua 2024-01-15 9:11 ` Sean Rhodes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox