Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite loop if structure length is 0
Please add the the BZ link to the commit message as blow format: -- Pkg/Module: message title REF: BZ_Link Commit message. Signed-off-by: Name -- With that done, Reviewed-by: Zhichao Gao Thanks, Zhichao > -Original Message- > From: devel@edk2.groups.io On Behalf Of Krzysztof > Koch > Sent: Monday, February 17, 2020 11:39 PM > To: devel@edk2.groups.io; Krzysztof Koch ; Gao, > Liming > Cc: Ni, Ray ; Gao, Zhichao ; Sami > Mujawar ; Matteo Carlini > ; nd > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite > loop if structure length is 0 > > Hi Liming, > > The BZ is: https://bugzilla.tianocore.org/show_bug.cgi?id=2534 > Please let me know if I should change something. > > Kind regards, > Krzysztof > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Krzysztof > Koch via Groups.Io > Sent: Monday, February 17, 2020 15:23 > To: devel@edk2.groups.io; liming@intel.com > Cc: Ni, Ray ; Gao, Zhichao ; Sami > Mujawar ; Matteo Carlini > ; nd > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite > loop if structure length is 0 > > Hi Liming, > > I haven't created a BZ yet, shall I create one? It would be great if the patch > makes it to the stable tag. > > Over the last few months I added some security features to acpiview. They make > this debug tool less sensitive to exploits from ACPI tables. This patch > completes > my efforts in making the tool more reliable. > > Kind regards, > Krzysztof > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Liming Gao > via Groups.Io > Sent: Monday, February 17, 2020 15:11 > To: devel@edk2.groups.io; Krzysztof Koch > Cc: Ni, Ray ; Gao, Zhichao ; Sami > Mujawar ; Matteo Carlini > ; nd > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite > loop if structure length is 0 > > Krzysztof: > Is there one BZ for this issue? Does this patch catch to this edk2 stable > tag > 202002? > > Thanks > Liming > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of > > Krzysztof Koch > > Sent: Friday, February 14, 2020 9:59 PM > > To: devel@edk2.groups.io > > Cc: Ni, Ray ; Gao, Zhichao ; > > sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com > > Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent > > infinite loop if structure length is 0 > > > > Extend validation of ACPI structure lengths which are read from the > > ACPI table being parsed. Additionally check if the structure 'Length' > > field value is positive. If not, stop parsing the faulting table. > > > > Some ACPI tables define internal structures of variable size. The > > 'Length' field inside the substructure is used to update a pointer > > used for table traversal. If the byte-length of the structure is equal > > to 0, acpiview can enter an infinite loop. This condition can occur > > if, for example, the zero-allocated ACPI table buffer is not fully > > populated. > > This is typically a bug on the ACPI table writer side. > > > > In short, this method helps acpiview recover gracefully from a > > zero-valued ACPI structure length. > > > > Signed-off-by: Krzysztof Koch > > --- > > > > Changes can be seen at: > > https://github.com/KrzysztofKoch1/edk2/tree/612_acpiview_prevent_inf_l > > oops_v1 > > > > Notes: > > v1: > > - prevent infinite loops in acpiview parsers [Krzysztof] > > > > > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c > > | 15 ++- > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c > > | 13 - > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c > > | 14 +- > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c > > | 28 ++-- > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c > > | 15 ++- > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c > > | 14 +- > > 6 files changed, 47 insertions(+), 52 deletions(-) > > > > diff --git > > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser > > .c > > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser > > .c index > > > 0f730a306a94329a23fbaf54b59f1833b44616ba..9df111ecaa7d7a703a13a39c24 > 3e > > d78b9f12ee97 100644 > > --- > > a/ShellPkg/Library/UefiShellAcpiViewCommandLib
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite loop if structure length is 0
Hi Liming,
The BZ is: https://bugzilla.tianocore.org/show_bug.cgi?id=2534
Please let me know if I should change something.
Kind regards,
Krzysztof
-Original Message-
From: devel@edk2.groups.io On Behalf Of Krzysztof Koch
via Groups.Io
Sent: Monday, February 17, 2020 15:23
To: devel@edk2.groups.io; liming@intel.com
Cc: Ni, Ray ; Gao, Zhichao ; Sami
Mujawar ; Matteo Carlini ; nd
Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite
loop if structure length is 0
Hi Liming,
I haven't created a BZ yet, shall I create one? It would be great if the patch
makes it to the stable tag.
Over the last few months I added some security features to acpiview. They make
this debug tool less sensitive to exploits from ACPI tables. This patch
completes my efforts in making the tool more reliable.
Kind regards,
Krzysztof
-Original Message-
From: devel@edk2.groups.io On Behalf Of Liming Gao via
Groups.Io
Sent: Monday, February 17, 2020 15:11
To: devel@edk2.groups.io; Krzysztof Koch
Cc: Ni, Ray ; Gao, Zhichao ; Sami
Mujawar ; Matteo Carlini ; nd
Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite
loop if structure length is 0
Krzysztof:
Is there one BZ for this issue? Does this patch catch to this edk2 stable tag
202002?
Thanks
Liming
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of
> Krzysztof Koch
> Sent: Friday, February 14, 2020 9:59 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Gao, Zhichao ;
> sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com
> Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent
> infinite loop if structure length is 0
>
> Extend validation of ACPI structure lengths which are read from the
> ACPI table being parsed. Additionally check if the structure 'Length'
> field value is positive. If not, stop parsing the faulting table.
>
> Some ACPI tables define internal structures of variable size. The
> 'Length' field inside the substructure is used to update a pointer
> used for table traversal. If the byte-length of the structure is equal
> to 0, acpiview can enter an infinite loop. This condition can occur
> if, for example, the zero-allocated ACPI table buffer is not fully populated.
> This is typically a bug on the ACPI table writer side.
>
> In short, this method helps acpiview recover gracefully from a
> zero-valued ACPI structure length.
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/612_acpiview_prevent_inf_l
> oops_v1
>
> Notes:
> v1:
> - prevent infinite loops in acpiview parsers [Krzysztof]
>
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> | 15 ++-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> | 13 -
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> | 14 +-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
> | 28 ++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> | 15 ++-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> | 14 +-
> 6 files changed, 47 insertions(+), 52 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> .c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> .c index
> 0f730a306a94329a23fbaf54b59f1833b44616ba..9df111ecaa7d7a703a13a39c243e
> d78b9f12ee97 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> .c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Pa
> +++ rser.c
> @@ -1,7 +1,7 @@
> /** @file
>DBG2 table parser
>
> - Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> + Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
>@par Reference(s):
> @@ -282,15 +282,16 @@ ParseAcpiDbg2 (
>return;
> }
>
> -// Make sure the Debug Device Information structure lies inside the
> table.
> -if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {
> +// Validate Debug Device Information Structure length
> +if ((*DbgDevInfoLen == 0) ||
> +((Offset + (*DbgDevInfoLen)) > AcpiTableLength)) {
>IncrementErrorCount ();
>Print (
> -L"ERROR: Invalid Debug Device Information structure length. " \
> - L"DbgDevInfoLen = %d. RemainingTableBufferLength = %d. " \
> - L"DBG2 parsing aborted.\n",
> +L"ERROR: Invalid Debug Device Information Structure length. " \
>
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite loop if structure length is 0
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
-Original Message-
From: Krzysztof Koch
Sent: 14 February 2020 13:59
To: devel@edk2.groups.io
Cc: ray...@intel.com; zhichao@intel.com; Sami Mujawar
; Matteo Carlini ; nd
Subject: [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite loop if structure
length is 0
Extend validation of ACPI structure lengths which are read from the ACPI table
being parsed. Additionally check if the structure 'Length'
field value is positive. If not, stop parsing the faulting table.
Some ACPI tables define internal structures of variable size. The 'Length'
field inside the substructure is used to update a pointer used for table
traversal. If the byte-length of the structure is equal to 0, acpiview can
enter an infinite loop. This condition can occur if, for example, the
zero-allocated ACPI table buffer is not fully populated.
This is typically a bug on the ACPI table writer side.
In short, this method helps acpiview recover gracefully from a zero-valued ACPI
structure length.
Signed-off-by: Krzysztof Koch
---
Changes can be seen at:
https://github.com/KrzysztofKoch1/edk2/tree/612_acpiview_prevent_inf_loops_v1
Notes:
v1:
- prevent infinite loops in acpiview parsers [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c | 15
++-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 13
-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 14
+-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 28
++--
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 15
++-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 14
+-
6 files changed, 47 insertions(+), 52 deletions(-)
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
index
0f730a306a94329a23fbaf54b59f1833b44616ba..9df111ecaa7d7a703a13a39c243ed78b9f12ee97
100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Pars
+++ er.c
@@ -1,7 +1,7 @@
/** @file
DBG2 table parser
- Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@par Reference(s):
@@ -282,15 +282,16 @@ ParseAcpiDbg2 (
return;
}
-// Make sure the Debug Device Information structure lies inside the table.
-if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {
+// Validate Debug Device Information Structure length
+if ((*DbgDevInfoLen == 0) ||
+((Offset + (*DbgDevInfoLen)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
-L"ERROR: Invalid Debug Device Information structure length. " \
- L"DbgDevInfoLen = %d. RemainingTableBufferLength = %d. " \
- L"DBG2 parsing aborted.\n",
+L"ERROR: Invalid Debug Device Information Structure length. " \
+ L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
*DbgDevInfoLen,
-AcpiTableLength - Offset
+Offset,
+AcpiTableLength
);
return;
}
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index
699a55b549ec3fa61bbd156898821055dc019199..bdd30ff45c61142c071ead63a27babab8998721b
100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtPars
+++ er.c
@@ -1,7 +1,7 @@
/** @file
GTDT table parser
- Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@par Reference(s):
@@ -327,15 +327,16 @@ ParseAcpiGtdt (
return;
}
-// Make sure the Platform Timer is inside the table.
-if ((Offset + *PlatformTimerLength) > AcpiTableLength) {
+// Validate Platform Timer Structure length
+if ((*PlatformTimerLength == 0) ||
+((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
L"ERROR: Invalid Platform Timer Structure length. " \
- L"PlatformTimerLength = %d. RemainingTableBufferLength = %d. " \
- L"GTDT parsing aborted.\n",
+ L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
*PlatformTimerLength,
-AcpiTableLength - Offset
+Offset,
+AcpiTableLength
);
return;
}
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite loop if structure length is 0
Krzysztof:
Yes. Please create one BZ for this issue.
Thanks
Liming
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Krzysztof Koch
> Sent: Monday, February 17, 2020 11:23 PM
> To: devel@edk2.groups.io; Gao, Liming
> Cc: Ni, Ray ; Gao, Zhichao ; Sami
> Mujawar ; Matteo Carlini
> ; nd
> Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite
> loop if structure length is 0
>
> Hi Liming,
>
> I haven't created a BZ yet, shall I create one? It would be great if the
> patch makes it to the stable tag.
>
> Over the last few months I added some security features to acpiview. They
> make this debug tool less sensitive to exploits from ACPI
> tables. This patch completes my efforts in making the tool more reliable.
>
> Kind regards,
> Krzysztof
>
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Liming Gao via
> Groups.Io
> Sent: Monday, February 17, 2020 15:11
> To: devel@edk2.groups.io; Krzysztof Koch
> Cc: Ni, Ray ; Gao, Zhichao ; Sami
> Mujawar ; Matteo Carlini
> ; nd
> Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite
> loop if structure length is 0
>
> Krzysztof:
> Is there one BZ for this issue? Does this patch catch to this edk2 stable
> tag 202002?
>
> Thanks
> Liming
> > -Original Message-
> > From: devel@edk2.groups.io On Behalf Of
> > Krzysztof Koch
> > Sent: Friday, February 14, 2020 9:59 PM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray ; Gao, Zhichao ;
> > sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com
> > Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent
> > infinite loop if structure length is 0
> >
> > Extend validation of ACPI structure lengths which are read from the
> > ACPI table being parsed. Additionally check if the structure 'Length'
> > field value is positive. If not, stop parsing the faulting table.
> >
> > Some ACPI tables define internal structures of variable size. The
> > 'Length' field inside the substructure is used to update a pointer
> > used for table traversal. If the byte-length of the structure is equal
> > to 0, acpiview can enter an infinite loop. This condition can occur
> > if, for example, the zero-allocated ACPI table buffer is not fully
> > populated.
> > This is typically a bug on the ACPI table writer side.
> >
> > In short, this method helps acpiview recover gracefully from a
> > zero-valued ACPI structure length.
> >
> > Signed-off-by: Krzysztof Koch
> > ---
> >
> > Changes can be seen at:
> > https://github.com/KrzysztofKoch1/edk2/tree/612_acpiview_prevent_inf_l
> > oops_v1
> >
> > Notes:
> > v1:
> > - prevent infinite loops in acpiview parsers [Krzysztof]
> >
> >
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> > | 15 ++-
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> > | 13 -
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> > | 14 +-
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
> > | 28 ++--
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> > | 15 ++-
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> > | 14 +-
> > 6 files changed, 47 insertions(+), 52 deletions(-)
> >
> > diff --git
> > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> > .c
> > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> > .c index
> > 0f730a306a94329a23fbaf54b59f1833b44616ba..9df111ecaa7d7a703a13a39c243e
> > d78b9f12ee97 100644
> > ---
> > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> > .c
> > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Pa
> > +++ rser.c
> > @@ -1,7 +1,7 @@
> > /** @file
> >DBG2 table parser
> >
> > - Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> > + Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
> >SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >@par Reference(s):
> > @@ -282,15 +282,16 @@ ParseAcpiDbg2 (
> >return;
> > }
> >
> > -// Make sure the Debug Device Information structure lies inside the
> > table.
> > -if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {
> > +// Validate Debug Device Information
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite loop if structure length is 0
Hi Liming,
I haven't created a BZ yet, shall I create one? It would be great if the patch
makes it to the stable tag.
Over the last few months I added some security features to acpiview. They make
this debug tool less sensitive to exploits from ACPI tables. This patch
completes my efforts in making the tool more reliable.
Kind regards,
Krzysztof
-Original Message-
From: devel@edk2.groups.io On Behalf Of Liming Gao via
Groups.Io
Sent: Monday, February 17, 2020 15:11
To: devel@edk2.groups.io; Krzysztof Koch
Cc: Ni, Ray ; Gao, Zhichao ; Sami
Mujawar ; Matteo Carlini ; nd
Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite
loop if structure length is 0
Krzysztof:
Is there one BZ for this issue? Does this patch catch to this edk2 stable tag
202002?
Thanks
Liming
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of
> Krzysztof Koch
> Sent: Friday, February 14, 2020 9:59 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Gao, Zhichao ;
> sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com
> Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent
> infinite loop if structure length is 0
>
> Extend validation of ACPI structure lengths which are read from the
> ACPI table being parsed. Additionally check if the structure 'Length'
> field value is positive. If not, stop parsing the faulting table.
>
> Some ACPI tables define internal structures of variable size. The
> 'Length' field inside the substructure is used to update a pointer
> used for table traversal. If the byte-length of the structure is equal
> to 0, acpiview can enter an infinite loop. This condition can occur
> if, for example, the zero-allocated ACPI table buffer is not fully populated.
> This is typically a bug on the ACPI table writer side.
>
> In short, this method helps acpiview recover gracefully from a
> zero-valued ACPI structure length.
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/612_acpiview_prevent_inf_l
> oops_v1
>
> Notes:
> v1:
> - prevent infinite loops in acpiview parsers [Krzysztof]
>
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> | 15 ++-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> | 13 -
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> | 14 +-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
> | 28 ++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> | 15 ++-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> | 14 +-
> 6 files changed, 47 insertions(+), 52 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> .c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> .c index
> 0f730a306a94329a23fbaf54b59f1833b44616ba..9df111ecaa7d7a703a13a39c243e
> d78b9f12ee97 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser
> .c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Pa
> +++ rser.c
> @@ -1,7 +1,7 @@
> /** @file
>DBG2 table parser
>
> - Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> + Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
>@par Reference(s):
> @@ -282,15 +282,16 @@ ParseAcpiDbg2 (
>return;
> }
>
> -// Make sure the Debug Device Information structure lies inside the
> table.
> -if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {
> +// Validate Debug Device Information Structure length
> +if ((*DbgDevInfoLen == 0) ||
> +((Offset + (*DbgDevInfoLen)) > AcpiTableLength)) {
>IncrementErrorCount ();
>Print (
> -L"ERROR: Invalid Debug Device Information structure length. " \
> - L"DbgDevInfoLen = %d. RemainingTableBufferLength = %d. " \
> - L"DBG2 parsing aborted.\n",
> +L"ERROR: Invalid Debug Device Information Structure length. " \
> + L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> *DbgDevInfoLen,
> -AcpiTableLength - Offset
> +Offset,
> +AcpiTableLength
> );
>return;
> }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c index
> 699a55b549ec3fa61bbd156898821055dc019199..bdd30ff45c61142c
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite loop if structure length is 0
Krzysztof:
Is there one BZ for this issue? Does this patch catch to this edk2 stable tag
202002?
Thanks
Liming
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Krzysztof Koch
> Sent: Friday, February 14, 2020 9:59 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Gao, Zhichao ;
> sami.muja...@arm.com; matteo.carl...@arm.com;
> n...@arm.com
> Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Prevent infinite
> loop if structure length is 0
>
> Extend validation of ACPI structure lengths which are read from the
> ACPI table being parsed. Additionally check if the structure 'Length'
> field value is positive. If not, stop parsing the faulting table.
>
> Some ACPI tables define internal structures of variable size. The
> 'Length' field inside the substructure is used to update a pointer used
> for table traversal. If the byte-length of the structure is equal to 0,
> acpiview can enter an infinite loop. This condition can occur if, for
> example, the zero-allocated ACPI table buffer is not fully populated.
> This is typically a bug on the ACPI table writer side.
>
> In short, this method helps acpiview recover gracefully from a
> zero-valued ACPI structure length.
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/612_acpiview_prevent_inf_loops_v1
>
> Notes:
> v1:
> - prevent infinite loops in acpiview parsers [Krzysztof]
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c | 15
> ++-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 13
> -
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 14
> +-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 28
> ++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 15
> ++-
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 14
> +-
> 6 files changed, 47 insertions(+), 52 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> index
> 0f730a306a94329a23fbaf54b59f1833b44616ba..9df111ecaa7d7a703a13a39c243ed78b9f12ee97
> 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> @@ -1,7 +1,7 @@
> /** @file
>DBG2 table parser
>
> - Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> + Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
>@par Reference(s):
> @@ -282,15 +282,16 @@ ParseAcpiDbg2 (
>return;
> }
>
> -// Make sure the Debug Device Information structure lies inside the
> table.
> -if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {
> +// Validate Debug Device Information Structure length
> +if ((*DbgDevInfoLen == 0) ||
> +((Offset + (*DbgDevInfoLen)) > AcpiTableLength)) {
>IncrementErrorCount ();
>Print (
> -L"ERROR: Invalid Debug Device Information structure length. " \
> - L"DbgDevInfoLen = %d. RemainingTableBufferLength = %d. " \
> - L"DBG2 parsing aborted.\n",
> +L"ERROR: Invalid Debug Device Information Structure length. " \
> + L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
> *DbgDevInfoLen,
> -AcpiTableLength - Offset
> +Offset,
> +AcpiTableLength
> );
>return;
> }
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> index
> 699a55b549ec3fa61bbd156898821055dc019199..bdd30ff45c61142c071ead63a27babab8998721b
> 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> @@ -1,7 +1,7 @@
> /** @file
>GTDT table parser
>
> - Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> + Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
>
>@par Reference(s):
> @@ -327,15 +327,16 @@ ParseAcpiGtdt (
>return;
> }
>
> -// Make sure the Platform Timer is inside the table.
> -if ((Offset + *PlatformTimerLength) > AcpiTableLength) {
> +// Validate Platform Timer Structure length
> +if ((*PlatformTimerLength == 0) ||
> +((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {
>IncrementErrorCount ();
>Print (
> L"ERROR: Invalid Platform Timer Structure length. " \
> - L"PlatformTimerLength = %d. RemainingTableBufferLength = %d. " \
> - L"GTDT parsing aborted.\n",
> + L"Length = %d. Offset = %d.
