* [PATCH v4] MinPlatformPkg/TestPointCheckLib: Add check for pointers
@ 2019-09-18 6:34 Zhang, Shenglei
2019-09-18 6:57 ` [edk2-devel] " Chiu, Chasel
2019-09-19 4:14 ` Kubacki, Michael A
0 siblings, 2 replies; 3+ messages in thread
From: Zhang, Shenglei @ 2019-09-18 6:34 UTC (permalink / raw)
To: devel; +Cc: Michael Kubacki, Chasel Chiu, Nate DeSimone, Liming Gao
In DxeCheckBootVariable.c, add check for BootOrder and Variable
that return EFI_NOT_FOUND when they are NULL.
In DxeCheckGcd.c, add check for GcdIoMap to ensure it not NULL
when allocating memory to what it points to.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
v2: Update copyright
v3: Fix the coding style.
v4: Update the logic in DxeCheckBootVariable.c.
We directly return when BootOrder/Variable == NULL, in previous versions.
.../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 6 +++---
.../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 8 +++++---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
index 85bd5b3d..06a40505 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
@@ -1,6 +1,6 @@
/** @file
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -130,7 +130,7 @@ TestPointCheckLoadOptionVariable (
for (ListIndex = 0; ListIndex < sizeof(mLoadOptionVariableList)/sizeof(mLoadOptionVariableList[0]); ListIndex++) {
UnicodeSPrint (BootOrderName, sizeof(BootOrderName), L"%sOrder", mLoadOptionVariableList[ListIndex]);
Status = GetVariable2 (BootOrderName, &gEfiGlobalVariableGuid, (VOID **)&BootOrder, &OrderSize);
- if (EFI_ERROR(Status)) {
+ if (EFI_ERROR(Status)||(BootOrder == NULL)) {
continue;
}
for (Index = 0; Index < OrderSize/sizeof(CHAR16); Index++) {
@@ -222,7 +222,7 @@ TestPointCheckKeyOptionVariable (
for (Index = 0; ; Index++) {
UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", mKeyOptionVariableList[ListIndex], Index);
Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, &Variable, &Size);
- if (!EFI_ERROR(Status)) {
+ if (!EFI_ERROR(Status)&&(Variable != NULL)) {
DumpKeyOption (KeyOptionName, Variable, Size);
} else {
break;
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c
index 82709d44..28ca8382 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c
@@ -1,6 +1,6 @@
/** @file
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -241,11 +241,13 @@ TestPointDumpGcd (
}
}
if (GcdMemoryMap != NULL) {
- *GcdIoMap = AllocateCopyPool (NumberOfDescriptors * sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+ if (GcdIoMap != NULL) {
+ *GcdIoMap = AllocateCopyPool (NumberOfDescriptors * sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+ }
*GcdIoMapNumberOfDescriptors = NumberOfDescriptors;
}
}
-
+
if (DumpPrint) {
DEBUG ((DEBUG_INFO, "==== TestPointDumpGcd - Exit\n"));
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v4] MinPlatformPkg/TestPointCheckLib: Add check for pointers
2019-09-18 6:34 [PATCH v4] MinPlatformPkg/TestPointCheckLib: Add check for pointers Zhang, Shenglei
@ 2019-09-18 6:57 ` Chiu, Chasel
2019-09-19 4:14 ` Kubacki, Michael A
1 sibling, 0 replies; 3+ messages in thread
From: Chiu, Chasel @ 2019-09-18 6:57 UTC (permalink / raw)
To: devel@edk2.groups.io, Zhang, Shenglei
Cc: Kubacki, Michael A, Desimone, Nathaniel L, Gao, Liming
Some minor comments for coding style in below inline, you can correct them when pushing the commit.
With those coding style issues fixed, Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang,
> Shenglei
> Sent: Wednesday, September 18, 2019 2:34 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH v4] MinPlatformPkg/TestPointCheckLib: Add
> check for pointers
>
> In DxeCheckBootVariable.c, add check for BootOrder and Variable that
> return EFI_NOT_FOUND when they are NULL.
> In DxeCheckGcd.c, add check for GcdIoMap to ensure it not NULL when
> allocating memory to what it points to.
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
>
> v2: Update copyright
>
> v3: Fix the coding style.
>
> v4: Update the logic in DxeCheckBootVariable.c.
> We directly return when BootOrder/Variable == NULL, in previous
> versions.
>
> .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 6 +++---
> .../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 8 +++++---
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB
> ootVariable.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB
> ootVariable.c
> index 85bd5b3d..06a40505 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB
> ootVariable.c
> +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckBootVariable.c
> @@ -1,6 +1,6 @@
> /** @file
>
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
Please add space before and after '-' like below:
2017 - 2019
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -130,7 +130,7 @@ TestPointCheckLoadOptionVariable (
> for (ListIndex = 0; ListIndex <
> sizeof(mLoadOptionVariableList)/sizeof(mLoadOptionVariableList[0]);
> ListIndex++) {
> UnicodeSPrint (BootOrderName, sizeof(BootOrderName), L"%sOrder",
> mLoadOptionVariableList[ListIndex]);
> Status = GetVariable2 (BootOrderName, &gEfiGlobalVariableGuid,
> (VOID **)&BootOrder, &OrderSize);
> - if (EFI_ERROR(Status)) {
> + if (EFI_ERROR(Status)||(BootOrder == NULL)) {
Add space before and after '||' like below:
if (EFI_ERROR(Status) || (BootOrder == NULL)) {
> continue;
> }
> for (Index = 0; Index < OrderSize/sizeof(CHAR16); Index++) { @@ -222,7
> +222,7 @@ TestPointCheckKeyOptionVariable (
> for (Index = 0; ; Index++) {
> UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x",
> mKeyOptionVariableList[ListIndex], Index);
> Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid,
> &Variable, &Size);
> - if (!EFI_ERROR(Status)) {
> + if (!EFI_ERROR(Status)&&(Variable != NULL)) {
Add space before and after '&&' like below:
if (!EFI_ERROR(Status) && (Variable != NULL)) {
> DumpKeyOption (KeyOptionName, Variable, Size);
> } else {
> break;
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG
> cd.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG
> cd.c
> index 82709d44..28ca8382 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG
> cd.c
> +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckGcd.c
> @@ -1,6 +1,6 @@
> /** @file
>
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -241,11 +241,13 @@ TestPointDumpGcd (
> }
> }
> if (GcdMemoryMap != NULL) {
> - *GcdIoMap = AllocateCopyPool (NumberOfDescriptors *
> sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
> + if (GcdIoMap != NULL) {
> + *GcdIoMap = AllocateCopyPool (NumberOfDescriptors *
> sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
> + }
> *GcdIoMapNumberOfDescriptors = NumberOfDescriptors;
> }
> }
> -
> +
> if (DumpPrint) {
> DEBUG ((DEBUG_INFO, "==== TestPointDumpGcd - Exit\n"));
> }
> --
> 2.18.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] MinPlatformPkg/TestPointCheckLib: Add check for pointers
2019-09-18 6:34 [PATCH v4] MinPlatformPkg/TestPointCheckLib: Add check for pointers Zhang, Shenglei
2019-09-18 6:57 ` [edk2-devel] " Chiu, Chasel
@ 2019-09-19 4:14 ` Kubacki, Michael A
1 sibling, 0 replies; 3+ messages in thread
From: Kubacki, Michael A @ 2019-09-19 4:14 UTC (permalink / raw)
To: Zhang, Shenglei, devel@edk2.groups.io
Cc: Chiu, Chasel, Desimone, Nathaniel L, Gao, Liming
1. PatchCheck flags commit message length
2. Please put [edk2-platforms] in the subject line for patches to the edk2-platforms repository
3. Please send a new patch with these changes and the changes for Chasel's feedback included
> -----Original Message-----
> From: Zhang, Shenglei <shenglei.zhang@intel.com>
> Sent: Tuesday, September 17, 2019 11:34 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH v4] MinPlatformPkg/TestPointCheckLib: Add check for
> pointers
>
> In DxeCheckBootVariable.c, add check for BootOrder and Variable that
> return EFI_NOT_FOUND when they are NULL.
> In DxeCheckGcd.c, add check for GcdIoMap to ensure it not NULL when
> allocating memory to what it points to.
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
>
> v2: Update copyright
>
> v3: Fix the coding style.
>
> v4: Update the logic in DxeCheckBootVariable.c.
> We directly return when BootOrder/Variable == NULL, in previous
> versions.
>
> .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 6 +++---
> .../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 8 +++++---
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> BootVariable.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> BootVariable.c
> index 85bd5b3d..06a40505 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> BootVariable.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckBootVariable.c
> @@ -1,6 +1,6 @@
> /** @file
>
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -130,7 +130,7 @@ TestPointCheckLoadOptionVariable (
> for (ListIndex = 0; ListIndex <
> sizeof(mLoadOptionVariableList)/sizeof(mLoadOptionVariableList[0]);
> ListIndex++) {
> UnicodeSPrint (BootOrderName, sizeof(BootOrderName), L"%sOrder",
> mLoadOptionVariableList[ListIndex]);
> Status = GetVariable2 (BootOrderName, &gEfiGlobalVariableGuid, (VOID
> **)&BootOrder, &OrderSize);
> - if (EFI_ERROR(Status)) {
> + if (EFI_ERROR(Status)||(BootOrder == NULL)) {
> continue;
> }
> for (Index = 0; Index < OrderSize/sizeof(CHAR16); Index++) { @@ -222,7
> +222,7 @@ TestPointCheckKeyOptionVariable (
> for (Index = 0; ; Index++) {
> UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x",
> mKeyOptionVariableList[ListIndex], Index);
> Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid,
> &Variable, &Size);
> - if (!EFI_ERROR(Status)) {
> + if (!EFI_ERROR(Status)&&(Variable != NULL)) {
> DumpKeyOption (KeyOptionName, Variable, Size);
> } else {
> break;
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> Gcd.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> Gcd.c
> index 82709d44..28ca8382 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> Gcd.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckGcd.c
> @@ -1,6 +1,6 @@
> /** @file
>
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -241,11 +241,13 @@ TestPointDumpGcd (
> }
> }
> if (GcdMemoryMap != NULL) {
> - *GcdIoMap = AllocateCopyPool (NumberOfDescriptors *
> sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
> + if (GcdIoMap != NULL) {
> + *GcdIoMap = AllocateCopyPool (NumberOfDescriptors *
> sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
> + }
> *GcdIoMapNumberOfDescriptors = NumberOfDescriptors;
> }
> }
> -
> +
> if (DumpPrint) {
> DEBUG ((DEBUG_INFO, "==== TestPointDumpGcd - Exit\n"));
> }
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-19 4:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-18 6:34 [PATCH v4] MinPlatformPkg/TestPointCheckLib: Add check for pointers Zhang, Shenglei
2019-09-18 6:57 ` [edk2-devel] " Chiu, Chasel
2019-09-19 4:14 ` Kubacki, Michael A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox