Hi Zhichao, I think you're right. I will submit a v2 patch that minimizes the amount of pointer validation required for parsing the remaining tables.
Kind regards, Krzysztof -----Original Message----- From: Gao, Zhichao <zhichao....@intel.com> Sent: Friday, August 16, 2019 8:35 To: Krzysztof Koch <krzysztof.k...@arm.com>; devel@edk2.groups.io Cc: Carsey, Jaben <jaben.car...@intel.com>; Ni, Ray <ray...@intel.com>; Sami Mujawar <sami.muja...@arm.com>; Matteo Carlini <matteo.carl...@arm.com>; nd <n...@arm.com> Subject: RE: [PATCH v1 03/11] ShellPkg: acpiview: FADT: Validate global pointer before use For FadtMinorRevision and X_DsdtAddress, I don't think they are required section. Maybe we should consider check the length before check them. As I know, the OVMF's FACP table doesn't have the section after flag. Thanks, Zhichao > -----Original Message----- > From: Krzysztof Koch [mailto:krzysztof.k...@arm.com] > Sent: Thursday, August 15, 2019 9:11 PM > To: devel@edk2.groups.io > Cc: Carsey, Jaben <jaben.car...@intel.com>; Ni, Ray > <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; > sami.muja...@arm.com; matteo.carl...@arm.com; n...@arm.com > Subject: [PATCH v1 03/11] ShellPkg: acpiview: FADT: Validate global > pointer before use > > Check if global pointers have been successfully updated before they > are used for further table parsing. > > Signed-off-by: Krzysztof Koch <krzysztof.k...@arm.com> > --- > > Notes: > v1: > - Test against NULL pointers [Krzysztof] > > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c > | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser. > c > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser. > c > index > e40c9ef8ee4b3285faf8c6edf3cb6236ee367397..e218e45926abced1096e75441 > e22108db7a3a811 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser. > c > +++ > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtPars > +++ er.c > @@ -203,6 +203,20 @@ ParseAcpiFadt ( > PARSER_PARAMS (FadtParser) > ); > > + // Check if the values used to control the parsing logic have been > + // successfully read. > + if ((DsdtAddress == NULL) || > + (FadtMinorRevision == NULL) || > + (X_DsdtAddress == NULL)) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Insufficient table length. AcpiTableLength = %d. " \ > + L"FADT parsing aborted.\n", > + AcpiTableLength > + ); > + return; > + } > + > if (Trace) { > Print (L"\nSummary:\n"); > PrintFieldName (2, L"FADT Version"); > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#45816): https://edk2.groups.io/g/devel/message/45816 Mute This Topic: https://groups.io/mt/32886566/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
