[PATCH v3 0/3] OvmfPkg: Check arguments for validity

[PATCH v3 0/3] OvmfPkg: Check arguments for validity

[Dimitrije Pavlov]

Some functions across OVMF don't check pointer arguments for
validity, which causes null pointer dereferences and crashes
in the SCT test suite.

This series adds checks to return EFI_INVALID_PARAMETER if a
pointer argument is NULL.

v3:
- Fix coding standard issues to pass CI checks [Ard]

v2: https://edk2.groups.io/g/devel/message/92443

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Sunny Wang <Sunny.Wang@arm.com>
Cc: Jeff Booher-Kaeding <Jeff.Booher-Kaeding@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

Reviewed-by: Sunny Wang <sunny.wang@arm.com>

Dimitrije Pavlov (3):
  OvmfPkg/PlatformDxe: Check ExtractConfig and RouteConfig arguments
  OvmfPkg/VirtioGpuDxe: Check QueryMode arguments
  OvmfPkg/VirtioFsDxe: Check GetDriverName arguments

 OvmfPkg/PlatformDxe/Platform.c      | 8 ++++++++
 OvmfPkg/VirtioFsDxe/DriverBinding.c | 4 ++++
 OvmfPkg/VirtioGpuDxe/Gop.c          | 5 ++++-
 3 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.37.2

[PATCH v3 1/3] OvmfPkg/PlatformDxe: Check ExtractConfig and RouteConfig arguments

[Dimitrije Pavlov]

The current implementation does not check if Progress or Results
pointers in ExtractConfig are NULL, or if Progress pointer in
RouteConfig is NULL. This causes the SCT test suite to crash.

Add a check to return EFI_INVALID_PARAMETER if any of these pointers
are NULL.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Sunny Wang <Sunny.Wang@arm.com>
Cc: Jeff Booher-Kaeding <Jeff.Booher-Kaeding@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

Signed-off-by: Dimitrije Pavlov <Dimitrije.Pavlov@arm.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
---
 OvmfPkg/PlatformDxe/Platform.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
index 4bf22712c78f..a6d459f3dfd7 100644
--- a/OvmfPkg/PlatformDxe/Platform.c
+++ b/OvmfPkg/PlatformDxe/Platform.c
@@ -232,6 +232,10 @@ ExtractConfig (
 
   DEBUG ((DEBUG_VERBOSE, "%a: Request=\"%s\"\n", __FUNCTION__, Request));
 
+  if ((Progress == NULL) || (Results == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   Status = PlatformConfigToFormState (&MainFormState);
   if (EFI_ERROR (Status)) {
     *Progress = Request;
@@ -340,6 +344,10 @@ RouteConfig (
     Configuration
     ));
 
+  if (Progress == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   //
   // the "read" step in RMW
   //
-- 
2.37.2

[PATCH v3 2/3] OvmfPkg/VirtioGpuDxe: Check QueryMode arguments

[Dimitrije Pavlov]

The current implementation does not check if Info or SizeInfo
pointers are NULL. This causes the SCT test suite to crash.

Add a check to return EFI_INVALID_PARAMETER if any of these
pointers are NULL.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Sunny Wang <Sunny.Wang@arm.com>
Cc: Jeff Booher-Kaeding <Jeff.Booher-Kaeding@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

Signed-off-by: Dimitrije Pavlov <Dimitrije.Pavlov@arm.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
---
 OvmfPkg/VirtioGpuDxe/Gop.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c
index 401db47672ec..16e92830d411 100644
--- a/OvmfPkg/VirtioGpuDxe/Gop.c
+++ b/OvmfPkg/VirtioGpuDxe/Gop.c
@@ -308,7 +308,10 @@ GopQueryMode (
 {
   EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *GopModeInfo;
 
-  if (ModeNumber >= This->Mode->MaxMode) {
+  if ((Info == NULL) ||
+      (SizeOfInfo == NULL) ||
+      (ModeNumber >= This->Mode->MaxMode))
+  {
     return EFI_INVALID_PARAMETER;
   }
 
-- 
2.37.2

[PATCH v3 3/3] OvmfPkg/VirtioFsDxe: Check GetDriverName arguments

[Dimitrije Pavlov]

The current implementation does not check if Language or DriverName
are NULL. This causes the SCT test suite to crash.

Add a check to return EFI_INVALID_PARAMETER if any of these pointers
are NULL.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Sunny Wang <Sunny.Wang@arm.com>
Cc: Jeff Booher-Kaeding <Jeff.Booher-Kaeding@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

Signed-off-by: Dimitrije Pavlov <Dimitrije.Pavlov@arm.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
---
 OvmfPkg/VirtioFsDxe/DriverBinding.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/OvmfPkg/VirtioFsDxe/DriverBinding.c b/OvmfPkg/VirtioFsDxe/DriverBinding.c
index 86eb9cf0ba51..69d2bb777089 100644
--- a/OvmfPkg/VirtioFsDxe/DriverBinding.c
+++ b/OvmfPkg/VirtioFsDxe/DriverBinding.c
@@ -218,6 +218,10 @@ VirtioFsGetDriverName (
   OUT CHAR16                        **DriverName
   )
 {
+  if ((Language == NULL) || (DriverName == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   if (AsciiStrCmp (Language, "en") != 0) {
     return EFI_UNSUPPORTED;
   }
-- 
2.37.2