* [PATCH v2 1/3] OvmfPkg/PlatformDxe: Check ExtractConfig and RouteConfig arguments
2022-08-15 16:31 [PATCH v2 0/3] OvmfPkg: Check arguments for validity Dimitrije Pavlov
@ 2022-08-15 16:31 ` Dimitrije Pavlov
2022-08-15 16:31 ` [PATCH v2 2/3] OvmfPkg/VirtioGpuDxe: Check QueryMode arguments Dimitrije Pavlov
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Dimitrije Pavlov @ 2022-08-15 16:31 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jiewen Yao, Liming Gao, Sunny Wang,
Jeff Booher-Kaeding, Samer El-Haj-Mahmoud
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>
---
OvmfPkg/PlatformDxe/Platform.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
index 4bf22712c78f..d7be2ab65efa 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] OvmfPkg/VirtioGpuDxe: Check QueryMode arguments
2022-08-15 16:31 [PATCH v2 0/3] OvmfPkg: Check arguments for validity Dimitrije Pavlov
2022-08-15 16:31 ` [PATCH v2 1/3] OvmfPkg/PlatformDxe: Check ExtractConfig and RouteConfig arguments Dimitrije Pavlov
@ 2022-08-15 16:31 ` Dimitrije Pavlov
2022-08-15 16:31 ` [PATCH v2 3/3] OvmfPkg/VirtioFsDxe: Check GetDriverName arguments Dimitrije Pavlov
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Dimitrije Pavlov @ 2022-08-15 16:31 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jiewen Yao, Liming Gao, Sunny Wang,
Jeff Booher-Kaeding, Samer El-Haj-Mahmoud
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>
---
OvmfPkg/VirtioGpuDxe/Gop.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c
index 401db47672ec..bb68b1cdc2bc 100644
--- a/OvmfPkg/VirtioGpuDxe/Gop.c
+++ b/OvmfPkg/VirtioGpuDxe/Gop.c
@@ -308,7 +308,9 @@ 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] OvmfPkg/VirtioFsDxe: Check GetDriverName arguments
2022-08-15 16:31 [PATCH v2 0/3] OvmfPkg: Check arguments for validity Dimitrije Pavlov
2022-08-15 16:31 ` [PATCH v2 1/3] OvmfPkg/PlatformDxe: Check ExtractConfig and RouteConfig arguments Dimitrije Pavlov
2022-08-15 16:31 ` [PATCH v2 2/3] OvmfPkg/VirtioGpuDxe: Check QueryMode arguments Dimitrije Pavlov
@ 2022-08-15 16:31 ` Dimitrije Pavlov
2022-08-15 16:35 ` [PATCH v2 0/3] OvmfPkg: Check arguments for validity Ard Biesheuvel
2022-08-16 11:39 ` Sunny Wang
4 siblings, 0 replies; 7+ messages in thread
From: Dimitrije Pavlov @ 2022-08-15 16:31 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jiewen Yao, Liming Gao, Sunny Wang,
Jeff Booher-Kaeding, Samer El-Haj-Mahmoud
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>
---
OvmfPkg/VirtioFsDxe/DriverBinding.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/OvmfPkg/VirtioFsDxe/DriverBinding.c b/OvmfPkg/VirtioFsDxe/DriverBinding.c
index 86eb9cf0ba51..3d80ff0f91f5 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] OvmfPkg: Check arguments for validity
2022-08-15 16:31 [PATCH v2 0/3] OvmfPkg: Check arguments for validity Dimitrije Pavlov
` (2 preceding siblings ...)
2022-08-15 16:31 ` [PATCH v2 3/3] OvmfPkg/VirtioFsDxe: Check GetDriverName arguments Dimitrije Pavlov
@ 2022-08-15 16:35 ` Ard Biesheuvel
2022-08-16 11:39 ` Sunny Wang
4 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2022-08-15 16:35 UTC (permalink / raw)
To: Dimitrije Pavlov, Liming Gao, Michael Kinney
Cc: devel, Jiewen Yao, Sunny Wang, Jeff Booher-Kaeding,
Samer El-Haj-Mahmoud
On Mon, 15 Aug 2022 at 18:31, Dimitrije Pavlov <dimitrije.pavlov@arm.com> wrote:
>
> 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.
>
> v2:
> - Add Liming Gao to Cc [Ard]
> - Turn individual patches into a series [Ard]
> - Fix issue with corrupted patches [Ard]
>
> 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>
>
> Dimitrije Pavlov (3):
> OvmfPkg/PlatformDxe: Check ExtractConfig and RouteConfig arguments
> OvmfPkg/VirtioGpuDxe: Check QueryMode arguments
> OvmfPkg/VirtioFsDxe: Check GetDriverName arguments
>
Thank you for the resend.
@Liming: these are all bug fixes that affect SCT results, so unless
there are any objections, I intend to merge these tomorrow (Tuesday).
Thanks,
Ard.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] OvmfPkg: Check arguments for validity
2022-08-15 16:31 [PATCH v2 0/3] OvmfPkg: Check arguments for validity Dimitrije Pavlov
` (3 preceding siblings ...)
2022-08-15 16:35 ` [PATCH v2 0/3] OvmfPkg: Check arguments for validity Ard Biesheuvel
@ 2022-08-16 11:39 ` Sunny Wang
2022-08-16 19:14 ` Ard Biesheuvel
4 siblings, 1 reply; 7+ messages in thread
From: Sunny Wang @ 2022-08-16 11:39 UTC (permalink / raw)
To: Dimitrije Pavlov, devel@edk2.groups.io
Cc: Ard Biesheuvel, Jiewen Yao, Liming Gao, Jeff Booher-Kaeding,
Samer El-Haj-Mahmoud, Sunny Wang
The series looks good to me.
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
-----Original Message-----
From: Dimitrije Pavlov <dimitrije.pavlov@arm.com>
Sent: 15 August 2022 17:31
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Jiewen Yao <jiewen.yao@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Sunny Wang <Sunny.Wang@arm.com>; Jeff Booher-Kaeding <Jeff.Booher-Kaeding@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: [PATCH v2 0/3] OvmfPkg: Check arguments for validity
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.
v2:
- Add Liming Gao to Cc [Ard]
- Turn individual patches into a series [Ard]
- Fix issue with corrupted patches [Ard]
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>
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 | 4 +++-
3 files changed, 15 insertions(+), 1 deletion(-)
--
2.37.2
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] OvmfPkg: Check arguments for validity
2022-08-16 11:39 ` Sunny Wang
@ 2022-08-16 19:14 ` Ard Biesheuvel
0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2022-08-16 19:14 UTC (permalink / raw)
To: Sunny Wang
Cc: Dimitrije Pavlov, devel@edk2.groups.io, Ard Biesheuvel,
Jiewen Yao, Liming Gao, Jeff Booher-Kaeding, Samer El-Haj-Mahmoud
On Tue, 16 Aug 2022 at 13:39, Sunny Wang <Sunny.Wang@arm.com> wrote:
>
> The series looks good to me.
> Reviewed-by: Sunny Wang <sunny.wang@arm.com>
>
> -----Original Message-----
> From: Dimitrije Pavlov <dimitrije.pavlov@arm.com>
> Sent: 15 August 2022 17:31
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Jiewen Yao <jiewen.yao@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Sunny Wang <Sunny.Wang@arm.com>; Jeff Booher-Kaeding <Jeff.Booher-Kaeding@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Subject: [PATCH v2 0/3] OvmfPkg: Check arguments for validity
>
> 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.
>
> v2:
> - Add Liming Gao to Cc [Ard]
> - Turn individual patches into a series [Ard]
> - Fix issue with corrupted patches [Ard]
>
> 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>
>
> Dimitrije Pavlov (3):
> OvmfPkg/PlatformDxe: Check ExtractConfig and RouteConfig arguments
> OvmfPkg/VirtioGpuDxe: Check QueryMode arguments
> OvmfPkg/VirtioFsDxe: Check GetDriverName arguments
>
I tried to push these but they failed in CI.
Could you please have a look and respin with the reported issues
addressed? Thanks.
https://github.com/tianocore/edk2/pull/3211
^ permalink raw reply [flat|nested] 7+ messages in thread