Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
OK. I got your point. 1. The length of debug device info structure indicate the correct length should be parsed. I'd prefer to see this as a backward compatibility as you mentioned. See below: The ACPI table may be updated to extend some of the table. For example, if DBG2->Debug Device Info structure is enlarged (add new field after AddressSizeOffset), we would update the DbgDevInforParser and that would cause an incorrect parsing of old version Debug device info structure. So it is required to get the length of the structure first. I am not sure if it is required for the acpi table header. 2. As you mentioned, using length to parse a structure may cause some variable not updated. I didn't consider that before. That's good point. If it is not updated, it would use its default value or the previous value that initialize thru the previous ParseAcpi. So I think it is required to get parsed value (return value of ParseAcpi) to judge if the variable is valid. For this patch, I have no comments. Reviewed-by: Zhichao Gao Thanks, Zhichao > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Krzysztof Koch > Sent: Tuesday, August 6, 2019 6:45 PM > To: devel@edk2.groups.io; Gao, Zhichao > Cc: Carsey, Jaben ; Ni, Ray ; > Sami Mujawar ; Matteo Carlini > ; nd > Subject: Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent > buffer overruns > > Hi Zhichao, > > Thanks for the feedback. I had a look at your code and I have put comments > inline (in GitHub) to describe why I think it does not achieve the same > functionality as the patch I've submitted. > > I copied my comments below as well, so that they're easier to access: > > """ > The ParseAcpi() call (Line 113) will parse either the entire > DbgDevInfoParser[] > array or as much data as there is left in the ACPI table buffer. I agree this > prevents buffer overruns with respect to the ACPI table buffer. However, > the parser now ignores the length of the Debug Device Information Structure > (loaded into the *DbgDevInfoLen variable) when dumping its contents. > > Here is an example: > If the DBG2 table buffer is 100-byte long, and the Debug Device Information > Structure is (let's say) located at offset 20 with a byte-size (as described > in > the 'Length' field) of only 10 bytes, then we have a problem. > > The DbgDevInfoParser[] array says that 22 bytes should be parsed, however, > the user-provided structure length is 10. I believe that only 10 bytes should > be parsed to reflect what an OS would do in this situation. > > This is why I created a new ACPI_PARSER array in my submitted patch to: > 1. Read the Length of the Debug Device Information Structure 2. Validate the > Length against the length of the DBG2 table buffer 3. Use the Length to > control how many statements from DbgDevInfoParser[] should be executed. > > If we print only as much data as the ACPI table writer has specified then any > errors in the 'Length' field are easier to detect. You can easily see that > some > data is missing and this is due to the 'Length' field having wrong value. > > Reading the 'Length' field before the whole structure is dumped is also > important for our acpiview implementation for the sake of backward > compatibility. As ACPI tables usually get updated by appending new fields to > existing structures. If someone provides us with a Length that matches the > old DBG2 version then we won't print the fields that got recently added to > DbgDevInfoParser[] due to a spec update. > > I understand there is still an issue of some variables not getting updated > correctly because we haven't parsed enough of the DbgDevInfoParser[], for > example, the AddrSizeOffset variable. But my next patch series adds code to > detect NULL pointers in all parsers. > """ > > Please let me know what you think. > > Kind regards, > > Krzysztof > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Gao, > Zhichao via Groups.Io > Sent: Tuesday, August 6, 2019 8:43 > To: devel@edk2.groups.io; Krzysztof Koch > Cc: Carsey, Jaben ; Ni, Ray ; > Sami Mujawar ; Matteo Carlini > ; nd > Subject: Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent > buffer overruns > > I got your point. How about this: > https://github.com/ZhichaoGao/edk2/commit/112a41255cb775f5ebede089b > 8b07ba7b836ec44 > I make a minor change of it. But I can't test it because I don't have a > platform > that implement DBG2 table. > > Thanks, > Zhichao > > > -Original Message----- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > > Krzysztof Koch > > Sent: Monday,
Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
I got your point. How about this:
https://github.com/ZhichaoGao/edk2/commit/112a41255cb775f5ebede089b8b07ba7b836ec44
I make a minor change of it. But I can't test it because I don't have a
platform that implement DBG2 table.
Thanks,
Zhichao
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Krzysztof Koch
> Sent: Monday, August 5, 2019 4:21 PM
> To: Gao, Zhichao ; devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray ;
> Sami Mujawar ; Matteo Carlini
> ; nd
> Subject: Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent
> buffer overruns
>
> Hi Zhichao,
>
> The reason why processing of the Debug Device Information Structure is split
> into:
> 1. loading the header
> 2. dumping the entire structure
>
> Is because we want to let the users control how much of the structure is
> dumped. This is important for backward compatibility of the acpiview tool
> with the ACPI specification (and other specs).
>
> New ACPI table fields are appended at the end of structures/tables. If, for
> example, we are asked to parse an old version of Debug Device Information
> Structure, the 'Length' field will tell us to ignore some of the newly added
> fields. These fields do not make sense in the context of an old version of the
> corresponding spec.
>
> The following code in Dbg2Parser.c:
>
> // Make sure the Debug Device Information structure lies inside the table.
> if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {
> IncrementErrorCount ();
> Print (
> L"ERROR: Invalid Debug Device Information structure length. " \
> L"DbgDevInfoLen = %d. RemainingTableBufferLength = %d. " \
> L"DBG2 parsing aborted.\n",
> *DbgDevInfoLen,
> AcpiTableLength - Offset
> );
> return;
> }
>
> Makes sure that the user-provided structure length won't result in a buffer
> overrun with respect to the DBG2 table buffer. This way we allow users to
> specify how much of the structure they want to parse while still preventing
> buffer overruns.
>
> In short, I'm not sure if getting rid of DbgDevInfoHeaderParser would work as
> you assume that the remaining table buffer length should be passed to
> ParseAcpi() as an argument, not the length of the Debug Device Information
> Structure. What do you think?
>
> Kind regards,
>
> Krzysztof
>
> -Original Message-
> From: Gao, Zhichao
> Sent: Monday, August 5, 2019 7:48
> To: Krzysztof Koch ; devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray ;
> Sami Mujawar ; Matteo Carlini
> ; nd
> Subject: RE: [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer
> overruns
>
> About DbgDevInfoHeaderParser and DbgDevInfoParser.
> This patch would parse same DbgDevInfo twice, one for getting length, the
> other for dumping structure info.
> How about the following?
> Add one parameter for DumpDbgDeviceInfo
>
> STATIC
> VOID
> EFIAPI
> DumpDbgDeviceInfo (
> IN UINT8* Ptr,
> OUT UINT32* Length
> )
>
> ==>
>
> STATIC
> VOID
> EFIAPI
> DumpDbgDeviceInfo (
> IN UINT8* Ptr,
> IN UINT32* Length // remain length of acpi struct to
> parse to make sure all operation is in a valid scope
> OUT UINT16* DbgDevInfoLength// return pointer dbgdevinfo length
> )
>
> Then we would not need an anditional DbgDevInfoHeaderParser and the
> header would be parsed for only once.
>
> Any better comments, please let me know.
>
> Thanks,
> Zhichao
>
> > -Original Message-
> > From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> > Sent: Thursday, August 1, 2019 4:44 PM
> > To: devel@edk2.groups.io
> > Cc: Carsey, Jaben ; Ni, Ray
> > ; Gao, Zhichao ;
> > sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com
> > Subject: [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer
> > overruns
> >
> > Modify the DBG2 table parsing logic to prevent reading past the ACPI
> > buffer lengths provided.
> >
> > Modify the signature of the DumpDbgDeviceInfo() function to make it
> > consistent with the ACPI structure processing functions in other
> > acpiview parsers. Now, the length of the Debug Device Information
> > Structure is read before the entire structure is dumped.
> >
> > This refactoring change makes it easier to stop reading beyond the
> > DBG2 table buffer if the Debug Device Information Structure Buffer
> > does not fit in the DBG2 buffer.
> >
> > For processing the first two fields of the Debug Device Information
> > Structure (to get the
Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
Hi Zhichao,
The reason why processing of the Debug Device Information Structure is split
into:
1. loading the header
2. dumping the entire structure
Is because we want to let the users control how much of the structure is
dumped. This is important for backward compatibility of the acpiview tool with
the ACPI specification (and other specs).
New ACPI table fields are appended at the end of structures/tables. If, for
example, we are asked to parse an old version of Debug Device Information
Structure, the 'Length' field will tell us to ignore some of the newly added
fields. These fields do not make sense in the context of an old version of the
corresponding spec.
The following code in Dbg2Parser.c:
// Make sure the Debug Device Information structure lies inside the table.
if ((Offset + *DbgDevInfoLen) > AcpiTableLength) {
IncrementErrorCount ();
Print (
L"ERROR: Invalid Debug Device Information structure length. " \
L"DbgDevInfoLen = %d. RemainingTableBufferLength = %d. " \
L"DBG2 parsing aborted.\n",
*DbgDevInfoLen,
AcpiTableLength - Offset
);
return;
}
Makes sure that the user-provided structure length won't result in a buffer
overrun with respect to the DBG2 table buffer. This way we allow users to
specify how much of the structure they want to parse while still preventing
buffer overruns.
In short, I'm not sure if getting rid of DbgDevInfoHeaderParser would work as
you assume that the remaining table buffer length should be passed to
ParseAcpi() as an argument, not the length of the Debug Device Information
Structure. What do you think?
Kind regards,
Krzysztof
-Original Message-
From: Gao, Zhichao
Sent: Monday, August 5, 2019 7:48
To: Krzysztof Koch ; devel@edk2.groups.io
Cc: Carsey, Jaben ; Ni, Ray ; Sami
Mujawar ; Matteo Carlini ; nd
Subject: RE: [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
About DbgDevInfoHeaderParser and DbgDevInfoParser.
This patch would parse same DbgDevInfo twice, one for getting length, the other
for dumping structure info.
How about the following?
Add one parameter for DumpDbgDeviceInfo
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
IN UINT8* Ptr,
OUT UINT32* Length
)
==>
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
IN UINT8* Ptr,
IN UINT32* Length// remain length of acpi struct to
parse to make sure all operation is in a valid scope
OUT UINT16* DbgDevInfoLength // return pointer dbgdevinfo length
)
Then we would not need an anditional DbgDevInfoHeaderParser and the header
would be parsed for only once.
Any better comments, please let me know.
Thanks,
Zhichao
> -Original Message-
> From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> Sent: Thursday, August 1, 2019 4:44 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray
> ; Gao, Zhichao ;
> sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com
> Subject: [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer
> overruns
>
> Modify the DBG2 table parsing logic to prevent reading past the ACPI
> buffer lengths provided.
>
> Modify the signature of the DumpDbgDeviceInfo() function to make it
> consistent with the ACPI structure processing functions in other
> acpiview parsers. Now, the length of the Debug Device Information
> Structure is read before the entire structure is dumped.
>
> This refactoring change makes it easier to stop reading beyond the
> DBG2 table buffer if the Debug Device Information Structure Buffer
> does not fit in the DBG2 buffer.
>
> For processing the first two fields of the Debug Device Information
> Structure (to get the length) a new ACPI_PARSER array is defined.
>
> References:
> - Microsoft Debug Port Table 2 (DBG2), December 10, 2015
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Notes:
> v1:
> - Prevent buffer overruns in DBG2 acpiview parser [Krzysztof]
>
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> | 141 +---
> 1 file changed, 92 insertions(+), 49 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> index
> c6929695a1032c57761ef85002d6c51b7800ce23..869e700b9beda4886bf7bc5ae
> 4ced3ab9a59efa3 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Pars
> +++ er.c
> @@ -64,10 +64,17 @@ STATIC CONST ACPI_PARSER Dbg2Parser[] = {
> (VOID**), NULL, NULL} };
>
> +/// An ACPI_PARSER array describing the debug device information
> +structure /// header.
> +STATIC CONST ACPI_PARSER DbgDevInfoHeaderParser[] = {
> + {L"Revision", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL}
> +};
> +
> /// An ACPI_PARSER array describing the debug device
Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
About DbgDevInfoHeaderParser and DbgDevInfoParser.
This patch would parse same DbgDevInfo twice, one for getting length, the other
for dumping structure info.
How about the following?
Add one parameter for DumpDbgDeviceInfo
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
IN UINT8* Ptr,
OUT UINT32* Length
)
==>
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
IN UINT8* Ptr,
IN UINT32* Length// remain length of acpi struct to
parse to make sure all operation is in a valid scope
OUT UINT16* DbgDevInfoLength // return pointer dbgdevinfo length
)
Then we would not need an anditional DbgDevInfoHeaderParser and the header
would be parsed for only once.
Any better comments, please let me know.
Thanks,
Zhichao
> -Original Message-
> From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> Sent: Thursday, August 1, 2019 4:44 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray ;
> Gao, Zhichao ; sami.muja...@arm.com;
> matteo.carl...@arm.com; n...@arm.com
> Subject: [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
>
> Modify the DBG2 table parsing logic to prevent reading past the ACPI buffer
> lengths provided.
>
> Modify the signature of the DumpDbgDeviceInfo() function to make it
> consistent with the ACPI structure processing functions in other acpiview
> parsers. Now, the length of the Debug Device Information Structure is read
> before the entire structure is dumped.
>
> This refactoring change makes it easier to stop reading beyond the
> DBG2 table buffer if the Debug Device Information Structure Buffer does not
> fit in the DBG2 buffer.
>
> For processing the first two fields of the Debug Device Information Structure
> (to get the length) a new ACPI_PARSER array is defined.
>
> References:
> - Microsoft Debug Port Table 2 (DBG2), December 10, 2015
>
> Signed-off-by: Krzysztof Koch
> ---
>
> Notes:
> v1:
> - Prevent buffer overruns in DBG2 acpiview parser [Krzysztof]
>
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> | 141 +---
> 1 file changed, 92 insertions(+), 49 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> index
> c6929695a1032c57761ef85002d6c51b7800ce23..869e700b9beda4886bf7bc5ae
> 4ced3ab9a59efa3 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parse
> r.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Pars
> +++ er.c
> @@ -64,10 +64,17 @@ STATIC CONST ACPI_PARSER Dbg2Parser[] = {
> (VOID**), NULL, NULL} };
>
> +/// An ACPI_PARSER array describing the debug device information
> +structure /// header.
> +STATIC CONST ACPI_PARSER DbgDevInfoHeaderParser[] = {
> + {L"Revision", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL} };
> +
> /// An ACPI_PARSER array describing the debug device information.
> STATIC CONST ACPI_PARSER DbgDevInfoParser[] = {
>{L"Revision", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
> - {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL},
> + {L"Length", 2, 1, L"%d", NULL, NULL, NULL, NULL},
>
>{L"Generic Address Registers Count", 1, 3, L"0x%x", NULL,
> (VOID**), NULL, NULL},
> @@ -93,76 +100,91 @@ STATIC CONST ACPI_PARSER DbgDevInfoParser[] = {
> /**
>This function parses the debug device information structure.
>
> - @param [in] Ptr Pointer to the start of the buffer.
> - @param [out] Length Pointer in which the length of the debug
> - device information is returned.
> + @param [in] Ptr Pointer to the start of the buffer.
> + @param [in] Length Length of the debug device information structure.
> **/
> STATIC
> VOID
> EFIAPI
> DumpDbgDeviceInfo (
> - IN UINT8* Ptr,
> - OUT UINT32* Length
> + IN UINT8* Ptr,
> + IN UINT16 Length
>)
> {
>UINT16 Index;
> - UINT8* DataPtr;
> - UINT32* AddrSize;
> -
> - // Parse the debug device info to get the Length
> - ParseAcpi (
> -FALSE,
> -0,
> -"Debug Device Info",
> -Ptr,
> -3, // Length is 2 bytes starting at offset 1
> -PARSER_PARAMS (DbgDevInfoParser)
> -);
> + UINT16 Offset;
>
>ParseAcpi (
> TRUE,
> 2,
> "Debug Device Info",
> Ptr,
> -*DbgDevInfoLen,
> +Length,
> PARSER_PARAMS (DbgDevInfoParser)
> );
>
> - // GAS and Address Size
> + // GAS
>Index = 0;
> - DataPtr = Ptr + (*BaseAddrRegOffset);
> - AddrSize = (UINT32*)(Ptr + (*AddrSizeOffset));
> - while (Index < (*GasCount)) {
> + Offset = *BaseAddrRegOffset;
> + while ((Index++ < *GasCount) &&
> + (Offset < Length)) {
> PrintFieldName (4, L"BaseAddressRegister");
> -DumpGasStruct (DataPtr, 4, GAS_LENGTH);
> +Offset += (UINT16)DumpGasStruct (
> +Ptr + Offset,
> +4,
> +
Re: [edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
Reviewed-by: Alexei Fedorov -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44775): https://edk2.groups.io/g/devel/message/44775 Mute This Topic: https://groups.io/mt/32676831/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 1/6] ShellPkg: acpiview: DBG2: Prevent buffer overruns
Modify the DBG2 table parsing logic to prevent reading past the ACPI
buffer lengths provided.
Modify the signature of the DumpDbgDeviceInfo() function to make it
consistent with the ACPI structure processing functions in other
acpiview parsers. Now, the length of the Debug Device Information
Structure is read before the entire structure is dumped.
This refactoring change makes it easier to stop reading beyond the
DBG2 table buffer if the Debug Device Information Structure Buffer
does not fit in the DBG2 buffer.
For processing the first two fields of the Debug Device Information
Structure (to get the length) a new ACPI_PARSER array is defined.
References:
- Microsoft Debug Port Table 2 (DBG2), December 10, 2015
Signed-off-by: Krzysztof Koch
---
Notes:
v1:
- Prevent buffer overruns in DBG2 acpiview parser [Krzysztof]
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c | 141
+---
1 file changed, 92 insertions(+), 49 deletions(-)
diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
index
c6929695a1032c57761ef85002d6c51b7800ce23..869e700b9beda4886bf7bc5ae4ced3ab9a59efa3
100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
@@ -64,10 +64,17 @@ STATIC CONST ACPI_PARSER Dbg2Parser[] = {
(VOID**), NULL, NULL}
};
+/// An ACPI_PARSER array describing the debug device information structure
+/// header.
+STATIC CONST ACPI_PARSER DbgDevInfoHeaderParser[] = {
+ {L"Revision", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
+ {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL}
+};
+
/// An ACPI_PARSER array describing the debug device information.
STATIC CONST ACPI_PARSER DbgDevInfoParser[] = {
{L"Revision", 1, 0, L"0x%x", NULL, NULL, NULL, NULL},
- {L"Length", 2, 1, L"%d", NULL, (VOID**), NULL, NULL},
+ {L"Length", 2, 1, L"%d", NULL, NULL, NULL, NULL},
{L"Generic Address Registers Count", 1, 3, L"0x%x", NULL,
(VOID**), NULL, NULL},
@@ -93,76 +100,91 @@ STATIC CONST ACPI_PARSER DbgDevInfoParser[] = {
/**
This function parses the debug device information structure.
- @param [in] Ptr Pointer to the start of the buffer.
- @param [out] Length Pointer in which the length of the debug
- device information is returned.
+ @param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the debug device information structure.
**/
STATIC
VOID
EFIAPI
DumpDbgDeviceInfo (
- IN UINT8* Ptr,
- OUT UINT32* Length
+ IN UINT8* Ptr,
+ IN UINT16 Length
)
{
UINT16 Index;
- UINT8* DataPtr;
- UINT32* AddrSize;
-
- // Parse the debug device info to get the Length
- ParseAcpi (
-FALSE,
-0,
-"Debug Device Info",
-Ptr,
-3, // Length is 2 bytes starting at offset 1
-PARSER_PARAMS (DbgDevInfoParser)
-);
+ UINT16 Offset;
ParseAcpi (
TRUE,
2,
"Debug Device Info",
Ptr,
-*DbgDevInfoLen,
+Length,
PARSER_PARAMS (DbgDevInfoParser)
);
- // GAS and Address Size
+ // GAS
Index = 0;
- DataPtr = Ptr + (*BaseAddrRegOffset);
- AddrSize = (UINT32*)(Ptr + (*AddrSizeOffset));
- while (Index < (*GasCount)) {
+ Offset = *BaseAddrRegOffset;
+ while ((Index++ < *GasCount) &&
+ (Offset < Length)) {
PrintFieldName (4, L"BaseAddressRegister");
-DumpGasStruct (DataPtr, 4, GAS_LENGTH);
+Offset += (UINT16)DumpGasStruct (
+Ptr + Offset,
+4,
+Length - Offset
+);
+ }
+
+ // Make sure the array of address sizes corresponding to each GAS fit in the
+ // Debug Device Information structure
+ if ((*AddrSizeOffset + (*GasCount * sizeof (UINT32))) > Length) {
+IncrementErrorCount ();
+Print (
+ L"ERROR: Invalid GAS count. GasCount = %d. RemainingBufferLength = %d. "
\
+L"Parsing of the Debug Device Information structure aborted.\n",
+ *GasCount,
+ Length - *AddrSizeOffset
+ );
+return;
+ }
+
+ // Address Size
+ Index = 0;
+ Offset = *AddrSizeOffset;
+ while ((Index++ < *GasCount) &&
+ (Offset < Length)) {
PrintFieldName (4, L"Address Size");
-Print (L"0x%x\n", AddrSize[Index]);
-DataPtr += GAS_LENGTH;
-Index++;
+Print (L"0x%x\n", *((UINT32*)(Ptr + Offset)));
+Offset += sizeof (UINT32);
}
// NameSpace String
Index = 0;
- DataPtr = Ptr + (*NameSpaceStringOffset);
+ Offset = *NameSpaceStringOffset;
PrintFieldName (4, L"NameSpace String");
- while (Index < (*NameSpaceStringLength)) {
-Print (L"%c", DataPtr[Index++]);
+ while ((Index++ < *NameSpaceStringLength) &&
+ (Offset < Length)) {
+Print (L"%c", *(Ptr + Offset));
+Offset++;
}
Print (L"\n");
// OEM Data
- Index = 0;
- DataPtr = Ptr +
