Re: [edk2-platform][PATCH v1 1/3] Platforms/RaspberryPi: Fix GOP parameter handling

["Andrei Warkentin" <awarkentin@vmware.com>]

[-- Attachment #1: Type: text/plain, Size: 3156 bytes --]

Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>

Not using mLastMode is definitely an improvement, thanks. Esp. since mLastMode isn't a very good name for the variable (i.e. it's not the last mode set, it's the last possible/valid mode index, based on the virtual resolution modes enabled via Pcd/HII)
________________________________
From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Sent: Tuesday, July 21, 2020 9:01 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Leif Lindholm <leif@nuviainc.com>; Pete Batard <pete@akeo.ie>; Andrei Warkentin <awarkentin@vmware.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>
Subject: [edk2-platform][PATCH v1 1/3] Platforms/RaspberryPi: Fix GOP parameter handling

Handle incorrect parameters passed to DisplayDxe GOP functions
QueryMode(), SetMode(), and Blt().

This fixes Blt_Conf and QueryMode_Conf failures
reported by SCT tests at:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4%2Fissues%2F73&amp;data=02%7C01%7Cawarkentin%40vmware.com%7C34c31d6924ca4f871d1b08d82de317b2%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637309800678733611&amp;sdata=3BSdjglhhk%2FqRrP0mDOxV8YydE4vf0FdDkMnzabV5qY%3D&amp;reserved=0

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Pete Batard <pete@akeo.ie>
Cc: Andrei Warkentin <awarkentin@vmware.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
---
 Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.c b/Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.c
index b880ca827bd6..80d0f7b2cb3c 100644
--- a/Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.c
+++ b/Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.c
@@ -1,5 +1,6 @@
 /** @file
  *
+ *  Copyright (c) 2020, ARM Limited. All rights reserved.
  *  Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
  *  Copyright (c) Microsoft Corporation. All rights reserved.
  *
@@ -170,7 +171,7 @@ DisplayQueryMode (
   EFI_STATUS Status;
   GOP_MODE_DATA *Mode;

-  if (ModeNumber > mLastMode) {
+  if (Info == NULL || SizeOfInfo == NULL || ModeNumber >= This->Mode->MaxMode) {
     return EFI_INVALID_PARAMETER;
   }

@@ -227,7 +228,7 @@ DisplaySetMode (
   EFI_PHYSICAL_ADDRESS FbBase;
   GOP_MODE_DATA *Mode = &mGopModeData[ModeNumber];

-  if (ModeNumber > mLastMode) {
+ if (ModeNumber >= This->Mode->MaxMode) {
     return EFI_UNSUPPORTED;
   }

@@ -299,6 +300,14 @@ DisplayBlt (
   UINT8 *VidBuf, *BltBuf, *VidBuf1;
   UINTN i;

+  if ((UINTN)BltOperation >= EfiGraphicsOutputBltOperationMax) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (Width == 0 || Height == 0) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   switch (BltOperation) {
   case EfiBltVideoFill:
     BltBuf = (UINT8*)BltBuffer;
@@ -349,7 +358,7 @@ DisplayBlt (
     break;

   default:
-    ASSERT_EFI_ERROR (EFI_SUCCESS);
+    return EFI_INVALID_PARAMETER;
     break;
   }

--
2.17.1


[-- Attachment #2: Type: text/html, Size: 5621 bytes --]