Split the Interrupt Controller Structure length validation in the acpiview UEFI shell tool into two logical parts: 1. Ensuring MADT table parser forward progress. 2. Preventing MADT table buffer overruns.
Also, make the condition for infinite loop detection applicable to all types of Interrupt Controller Structures (for all interrupt models which can be represented in MADT). Check if the controller length specified is shorter than the byte size of the first two fields ('Type' and 'Length') present in every valid Interrupt Controller Structure. Signed-off-by: Krzysztof Koch <krzysztof.k...@arm.com> --- Notes: v1: - split MADT structure length validation [Krzysztof] ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 30 ++++++++++++++------ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c index 338295d30e35c366a60505225cf57145a8e73d93..d80ebd1a2bae7a4acffe687ca5ee7b4090f0e223 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c @@ -260,16 +260,30 @@ ParseAcpiMadt ( PARSER_PARAMS (MadtInterruptControllerHeaderParser) ); - if (((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength) || - (*MadtInterruptControllerLength < 4)) { + // Make sure forward progress is made. + if (*MadtInterruptControllerLength < 2) { IncrementErrorCount (); Print ( - L"ERROR: Invalid Interrupt Controller Length," - L" Type = %d, Length = %d\n", - *MadtInterruptControllerType, - *MadtInterruptControllerLength - ); - break; + L"ERROR: Structure length is too small: " \ + L"MadtInterruptControllerLength = %d. " \ + L"MadtInterruptControllerType = %d. MADT parsing aborted.\n", + *MadtInterruptControllerLength, + *MadtInterruptControllerType + ); + return; + } + + // Make sure the MADT structure lies inside the table + if ((Offset + *MadtInterruptControllerLength) > AcpiTableLength) { + IncrementErrorCount (); + Print ( + L"ERROR: Invalid MADT structure length. " \ + L"MadtInterruptControllerLength = %d. " \ + L"RemainingTableBufferLength = %d. MADT parsing aborted.\n", + *MadtInterruptControllerLength, + AcpiTableLength - Offset + ); + return; } switch (*MadtInterruptControllerType) { -- 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44113): https://edk2.groups.io/g/devel/message/44113 Mute This Topic: https://groups.io/mt/32556349/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-