Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers
Hi Shenglei, You are right, *GcdIoMap will always be NULL but GcdIoMap may not be as the function does not exit early in the case of GcdIoMap being == NULL, sorry I misread. Please make sure you put a space between your closing parenthesis and your opening curly: if (GcdIoMap != NULL) { not... if (GcdIoMap != NULL){ Also, please fix the double semicolon and if statement whitespace: If (BootOrder == NULL) { return EFI_NOT_FOUND; } not... if(BootOrder == NULL) { return EFI_NOT_FOUND;; } Then you will get a reviewed-by. Thanks, Nate -Original Message- From: Zhang, Shenglei Sent: Monday, September 16, 2019 8:36 PM To: devel@edk2.groups.io; Desimone, Nathaniel L Cc: Kubacki, Michael A ; Chiu, Chasel ; Gao, Liming Subject: RE: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers Hi Nathaniel, > -Original Message- > From: Desimone, Nathaniel L > Sent: Saturday, September 14, 2019 6:31 AM > To: Zhang, Shenglei ; devel@edk2.groups.io > Cc: Kubacki, Michael A ; Chiu, Chasel > ; Gao, Liming > Subject: Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: > Add check for pointers > > Hi Shenglei, > > Looking at this patch more closely. There appear to be bugs... please > see below. Please fix this along with your poor use of semi-colons and > white-space. > > Thanks, > > Nate > > On 9/12/2019 11:54 AM, Nate DeSimone wrote: > > Your whitespace doesn't quite match the edk2 coding style > > guidelines, but > we can fix that during commit. > > > > Reviewed-by: Nate DeSimone > > > > -Original Message- > > From: Zhang, Shenglei > > Sent: Wednesday, September 11, 2019 7:55 PM > > To: devel@edk2.groups.io > > Cc: Kubacki, Michael A ; Chiu, Chasel > ; Desimone, Nathaniel L > ; Gao, Liming > > Subject: [PATCH] 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 > > Cc: Chasel Chiu > > Cc: Nate DeSimone > > Cc: Liming Gao > > Signed-off-by: Shenglei Zhang > > --- > > > > v2: Update copyright > > > > .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++- > > .../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 6 -- > > 2 files changed, 11 insertions(+), 3 deletions(-) > > > > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec > k > BootVariable.c > b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec > k > BootVariable.c > > index 85bd5b3d..98130683 100644 > > --- > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec > k > 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. > > +Copyright (c) 2017-2019, Intel Corporation. All rights > > +reserved. > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -130,6 +130,9 @@ 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(BootOrder == NULL) { > > + return EFI_NOT_FOUND;; > > +} > > if (EFI_ERROR(Status)) { > > continue; > > } > > @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( > > for (Index = 0; ; Index++) { > > UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), > > L"%s%04x", > mKeyOptionVariableList[ListIndex], Index); > > Status = GetVariable2 (KeyOptionName, > > &gEfiGlobalVariableGuid, > &Variable, &Size); > > + if(Variable == NULL) { > > +return EFI_NOT_FOUND;; > > + } > > if (!EFI_ERROR(Status)) { > > DumpKeyOption (KeyOptionName, Variable, Size); > > } else { > > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec > k > Gcd.c > b/Platfo
Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers
Hi Nathaniel, > -Original Message- > From: Desimone, Nathaniel L > Sent: Saturday, September 14, 2019 6:31 AM > To: Zhang, Shenglei ; devel@edk2.groups.io > Cc: Kubacki, Michael A ; Chiu, Chasel > ; Gao, Liming > Subject: Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add > check for pointers > > Hi Shenglei, > > Looking at this patch more closely. There appear to be bugs... please > see below. Please fix this along with your poor use of semi-colons and > white-space. > > Thanks, > > Nate > > On 9/12/2019 11:54 AM, Nate DeSimone wrote: > > Your whitespace doesn't quite match the edk2 coding style guidelines, but > we can fix that during commit. > > > > Reviewed-by: Nate DeSimone > > > > -Original Message- > > From: Zhang, Shenglei > > Sent: Wednesday, September 11, 2019 7:55 PM > > To: devel@edk2.groups.io > > Cc: Kubacki, Michael A ; Chiu, Chasel > ; Desimone, Nathaniel L > ; Gao, Liming > > Subject: [PATCH] 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 > > Cc: Chasel Chiu > > Cc: Nate DeSimone > > Cc: Liming Gao > > Signed-off-by: Shenglei Zhang > > --- > > > > v2: Update copyright > > > > .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++- > > .../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 6 -- > > 2 files changed, 11 insertions(+), 3 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..98130683 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. > > +Copyright (c) 2017-2019, Intel Corporation. All rights reserved. > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -130,6 +130,9 @@ 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(BootOrder == NULL) { > > + return EFI_NOT_FOUND;; > > +} > > if (EFI_ERROR(Status)) { > > continue; > > } > > @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( > > for (Index = 0; ; Index++) { > > UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", > mKeyOptionVariableList[ListIndex], Index); > > Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, > &Variable, &Size); > > + if(Variable == NULL) { > > +return EFI_NOT_FOUND;; > > + } > > if (!EFI_ERROR(Status)) { > > DumpKeyOption (KeyOptionName, Variable, Size); > > } else { > > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck > Gcd.c > b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck > Gcd.c > > index 82709d44..c90b37f2 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. > > +Copyright (c) 2017-2019, Intel Corporation. All rights reserved. > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -241,7 +241,9 @@ TestPointDumpGcd ( > > } > > } > > if (GcdMemoryMap != NULL) { > > - *GcdIoMap = AllocateCopyPool (NumberOfDescriptors * > sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap); > > + if (GcdIoMap != NULL){ > > +*GcdIoMap = AllocateCopyP
Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers
Hi Shenglei, Looking at this patch more closely. There appear to be bugs... please see below. Please fix this along with your poor use of semi-colons and white-space. Thanks, Nate On 9/12/2019 11:54 AM, Nate DeSimone wrote: > Your whitespace doesn't quite match the edk2 coding style guidelines, but we > can fix that during commit. > > Reviewed-by: Nate DeSimone > > -Original Message- > From: Zhang, Shenglei > Sent: Wednesday, September 11, 2019 7:55 PM > To: devel@edk2.groups.io > Cc: Kubacki, Michael A ; Chiu, Chasel > ; Desimone, Nathaniel L > ; Gao, Liming > Subject: [PATCH] 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 > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Liming Gao > Signed-off-by: Shenglei Zhang > --- > > v2: Update copyright > > .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++- > .../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 6 -- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c > > b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c > index 85bd5b3d..98130683 100644 > --- > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c > +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh > +++ eckBootVariable.c > @@ -1,6 +1,6 @@ > /** @file > > -Copyright (c) 2017, Intel Corporation. All rights reserved. > +Copyright (c) 2017-2019, Intel Corporation. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -130,6 +130,9 @@ 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(BootOrder == NULL) { > + return EFI_NOT_FOUND;; > +} > if (EFI_ERROR(Status)) { > continue; > } > @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( > for (Index = 0; ; Index++) { > UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", > mKeyOptionVariableList[ListIndex], Index); > Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, > &Variable, &Size); > + if(Variable == NULL) { > +return EFI_NOT_FOUND;; > + } > if (!EFI_ERROR(Status)) { > DumpKeyOption (KeyOptionName, Variable, Size); > } else { > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c > b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c > index 82709d44..c90b37f2 100644 > --- > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c > +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh > +++ eckGcd.c > @@ -1,6 +1,6 @@ > /** @file > > -Copyright (c) 2017, Intel Corporation. All rights reserved. > +Copyright (c) 2017-2019, Intel Corporation. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -241,7 +241,9 @@ 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); > + } GcdIoMap will always be NULL. Please see line 199 of this file. I believe your patch is introducing a new bug. > *GcdIoMapNumberOfDescriptors = NumberOfDescriptors; > } > } > -- > 2.18.0.windows.1 > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47233): https://edk2.groups.io/g/devel/message/47233 Mute This Topic: https://groups.io/mt/33110621/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers
Your whitespace doesn't quite match the edk2 coding style guidelines, but we can fix that during commit. Reviewed-by: Nate DeSimone -Original Message- From: Zhang, Shenglei Sent: Wednesday, September 11, 2019 7:55 PM To: devel@edk2.groups.io Cc: Kubacki, Michael A ; Chiu, Chasel ; Desimone, Nathaniel L ; Gao, Liming Subject: [PATCH] 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 Cc: Chasel Chiu Cc: Nate DeSimone Cc: Liming Gao Signed-off-by: Shenglei Zhang --- v2: Update copyright .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++- .../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 6 -- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c index 85bd5b3d..98130683 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh +++ eckBootVariable.c @@ -1,6 +1,6 @@ /** @file -Copyright (c) 2017, Intel Corporation. All rights reserved. +Copyright (c) 2017-2019, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -130,6 +130,9 @@ 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(BootOrder == NULL) { + return EFI_NOT_FOUND;; +} if (EFI_ERROR(Status)) { continue; } @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( for (Index = 0; ; Index++) { UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", mKeyOptionVariableList[ListIndex], Index); Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, &Variable, &Size); + if(Variable == NULL) { +return EFI_NOT_FOUND;; + } if (!EFI_ERROR(Status)) { DumpKeyOption (KeyOptionName, Variable, Size); } else { diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c index 82709d44..c90b37f2 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh +++ eckGcd.c @@ -1,6 +1,6 @@ /** @file -Copyright (c) 2017, Intel Corporation. All rights reserved. +Copyright (c) 2017-2019, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -241,7 +241,9 @@ 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; } } -- 2.18.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47198): https://edk2.groups.io/g/devel/message/47198 Mute This Topic: https://groups.io/mt/33110621/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] 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 Cc: Chasel Chiu Cc: Nate DeSimone Cc: Liming Gao Signed-off-by: Shenglei Zhang --- v2: Update copyright .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++- .../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 6 -- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c index 85bd5b3d..98130683 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. +Copyright (c) 2017-2019, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -130,6 +130,9 @@ 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(BootOrder == NULL) { + return EFI_NOT_FOUND;; +} if (EFI_ERROR(Status)) { continue; } @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( for (Index = 0; ; Index++) { UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", mKeyOptionVariableList[ListIndex], Index); Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, &Variable, &Size); + if(Variable == NULL) { +return EFI_NOT_FOUND;; + } if (!EFI_ERROR(Status)) { DumpKeyOption (KeyOptionName, Variable, Size); } else { diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c index 82709d44..c90b37f2 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. +Copyright (c) 2017-2019, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -241,7 +241,9 @@ 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; } } -- 2.18.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47157): https://edk2.groups.io/g/devel/message/47157 Mute This Topic: https://groups.io/mt/33110621/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers
Hi Shenglei, Please send a second patch series with the copyrights updated. Thanks, Nate -Original Message- From: Zhang, Shenglei Sent: Monday, September 2, 2019 5:35 AM To: devel@edk2.groups.io Cc: Kubacki, Michael A ; Chiu, Chasel ; Desimone, Nathaniel L ; Gao, Liming Subject: [PATCH] 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 Cc: Chasel Chiu Cc: Nate DeSimone Cc: Liming Gao Signed-off-by: Shenglei Zhang --- .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 6 ++ .../Test/Library/TestPointCheckLib/DxeCheckGcd.c| 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c index 85bd5b3d..a9af33a3 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh +++ eckBootVariable.c @@ -130,6 +130,9 @@ 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(BootOrder == NULL) { + return EFI_NOT_FOUND;; + } if (EFI_ERROR(Status)) { continue; } @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( for (Index = 0; ; Index++) { UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", mKeyOptionVariableList[ListIndex], Index); Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, &Variable, &Size); + if(Variable == NULL) { +return EFI_NOT_FOUND;; + } if (!EFI_ERROR(Status)) { DumpKeyOption (KeyOptionName, Variable, Size); } else { diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c index 82709d44..989606b6 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh +++ eckGcd.c @@ -241,7 +241,9 @@ 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; } } -- 2.18.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47086): https://edk2.groups.io/g/devel/message/47086 Mute This Topic: https://groups.io/mt/33110621/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers
Please extend copyright for files you touched, and one more update in below inline. > -Original Message- > From: Zhang, Shenglei > Sent: Monday, September 2, 2019 8:35 PM > To: devel@edk2.groups.io > Cc: Kubacki, Michael A ; Chiu, Chasel > ; Desimone, Nathaniel L > ; Gao, Liming > Subject: [PATCH] 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 > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Liming Gao > Signed-off-by: Shenglei Zhang > --- > .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 6 ++ > .../Test/Library/TestPointCheckLib/DxeCheckGcd.c| 4 +++- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB > ootVariable.c > b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB > ootVariable.c > index 85bd5b3d..a9af33a3 100644 > --- > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB > ootVariable.c > +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh > +++ eckBootVariable.c > @@ -130,6 +130,9 @@ 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(BootOrder == NULL) { > + return EFI_NOT_FOUND;; > + } Align indents with previous lines. > if (EFI_ERROR(Status)) { >continue; > } > @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( > for (Index = 0; ; Index++) { >UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", > mKeyOptionVariableList[ListIndex], Index); >Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, > &Variable, &Size); > + if(Variable == NULL) { > +return EFI_NOT_FOUND;; > + } >if (!EFI_ERROR(Status)) { > DumpKeyOption (KeyOptionName, Variable, Size); >} else { > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG > cd.c > b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG > cd.c > index 82709d44..989606b6 100644 > --- > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG > cd.c > +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh > +++ eckGcd.c > @@ -241,7 +241,9 @@ 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; > } >} > -- > 2.18.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46676): https://edk2.groups.io/g/devel/message/46676 Mute This Topic: https://groups.io/mt/33110621/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] 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 Cc: Chasel Chiu Cc: Nate DeSimone Cc: Liming Gao Signed-off-by: Shenglei Zhang --- .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 6 ++ .../Test/Library/TestPointCheckLib/DxeCheckGcd.c| 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c index 85bd5b3d..a9af33a3 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c @@ -130,6 +130,9 @@ 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(BootOrder == NULL) { + return EFI_NOT_FOUND;; + } if (EFI_ERROR(Status)) { continue; } @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( for (Index = 0; ; Index++) { UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", mKeyOptionVariableList[ListIndex], Index); Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, &Variable, &Size); + if(Variable == NULL) { +return EFI_NOT_FOUND;; + } if (!EFI_ERROR(Status)) { DumpKeyOption (KeyOptionName, Variable, Size); } else { diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c index 82709d44..989606b6 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c @@ -241,7 +241,9 @@ 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; } } -- 2.18.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46672): https://edk2.groups.io/g/devel/message/46672 Mute This Topic: https://groups.io/mt/33110621/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-