[edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Validate ACPI table 'Length' field
Gao, Zhichao
zhichao.gao at intel.com
Tue Feb 11 03:03:04 UTC 2020
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Krzysztof
> Koch
> Sent: Friday, January 31, 2020 12:20 AM
> To: devel at edk2.groups.io
> Cc: Ni, Ray <ray.ni at intel.com>; Gao, Zhichao <zhichao.gao at intel.com>;
> Matteo.Carlini at arm.com; sami.mujawar at arm.com; nd at arm.com
> Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Validate ACPI table
> 'Length' field
>
> Check if the ACPI table length, as reported in the ACPI table header, is big
> enough to fit at least the header itself.
>
> If not, report an error to the user and stop parsing the table in order to prevent
> buffer overruns.
>
> Signed-off-by: Krzysztof Koch <krzysztof.koch at arm.com>
> ---
>
> Changes can be seet at:
> https://github.com/KrzysztofKoch1/edk2/pull/new/650_add_checks_process_a
> cpi_table_v1
>
> Notes:
> v1:
> - Validate ACPI table length [Krzysztof]
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c | 22
> +++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> index
> d5500bcb2b4a55c7a69f45444aa49d36d2c1694f..0c93bca4fc0f7d2f105a765425
> 8e00f714fc1519 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI 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 **/
>
> @@ -176,6 +176,7 @@ ProcessAcpiTable (
> CONST UINT32* AcpiTableSignature;
> CONST UINT32* AcpiTableLength;
> CONST UINT8* AcpiTableRevision;
> + CONST UINT8* SignaturePtr;
> PARSE_ACPI_TABLE_PROC ParserProc;
>
> ParseAcpiHeader (
> @@ -193,6 +194,25 @@ ProcessAcpiTable (
>
> if (Trace) {
> DumpRaw (Ptr, *AcpiTableLength);
> +
> + /*
> + Do not process the ACPI table any further if the table length read
> + is invalid. The ACPI table should at least contain the table header.
> + */
The internal comment is suggested to use C++ style, refer to CSS 2.1 section 6.5.2.1:
For internal code comments, use C++ style (//) comment lines.
Thanks,
Zhichao
> + if (*AcpiTableLength < sizeof (EFI_ACPI_DESCRIPTION_HEADER)) {
> + SignaturePtr = (CONST UINT8*)AcpiTableSignature;
> + IncrementErrorCount ();
> + Print (
> + L"ERROR: Invalid %c%c%c%c table length. Length = %d\n",
> + SignaturePtr[0],
> + SignaturePtr[1],
> + SignaturePtr[2],
> + SignaturePtr[3],
> + *AcpiTableLength
> + );
> + return;
> + }
> +
> if (GetConsistencyChecking ()) {
> VerifyChecksum (TRUE, Ptr, *AcpiTableLength);
> }
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54174): https://edk2.groups.io/g/devel/message/54174
Mute This Topic: https://groups.io/mt/70272674/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list