* [PATCH v3 1/4] MdeModulePkg/EbcDebugger: Add check for Entry and RetEntry
2019-11-01 6:14 [PATCH v 3 0/4] MdeModulePkg: Add check and ASSERT for variables Zhang, Shenglei
@ 2019-11-01 6:14 ` Zhang, Shenglei
2019-11-01 6:14 ` [PATCH v3 2/4] MdeModulePkg/HiiDatabaseDxe: ASSERT StringPtr Zhang, Shenglei
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2019-11-01 6:14 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu
Entry and RetEntry might be NULL before used.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c | 2 +-
MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
index 8e305e4243a5..7b453fa98c2b 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
@@ -143,7 +143,7 @@ DebuggerDisplaySymbolAccrodingToAddress (
// Find the nearest symbol address
//
CandidateAddress = EbdFindSymbolAddress (Address, EdbMatchSymbolTypeNearestAddress, &Object, &Entry);
- if (CandidateAddress == 0 || CandidateAddress == (UINTN) -1) {
+ if (CandidateAddress == 0 || CandidateAddress == (UINTN) -1 || Entry == NULL) {
EDBPrint (L"Symbole at Address not found!\n");
return EFI_DEBUG_CONTINUE;
} else if (Address != CandidateAddress) {
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
index 85cc275c114b..90a9b9fbd7ee 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
@@ -2062,7 +2062,7 @@ EdbPrintSource (
&RetObject,
&RetEntry
);
- if (SymbolAddress == 0) {
+ if (SymbolAddress == 0 || RetEntry == NULL) {
return 0 ;
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/4] MdeModulePkg/HiiDatabaseDxe: ASSERT StringPtr
2019-11-01 6:14 [PATCH v 3 0/4] MdeModulePkg: Add check and ASSERT for variables Zhang, Shenglei
2019-11-01 6:14 ` [PATCH v3 1/4] MdeModulePkg/EbcDebugger: Add check for Entry and RetEntry Zhang, Shenglei
@ 2019-11-01 6:14 ` Zhang, Shenglei
2019-11-01 6:14 ` [PATCH v3 3/4] MdeModulePkg/EsrtDxe: Add check for EsrtRepository Zhang, Shenglei
2019-11-01 6:14 ` [PATCH v3 4/4] MdeModulePkg/SetupBrowserDxe: ASSERT GetBufferForValue(&Value) Zhang, Shenglei
3 siblings, 0 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2019-11-01 6:14 UTC (permalink / raw)
To: devel; +Cc: Dandan Bi, Eric Dong
The caller of CompareAndMergeDefaultString has checked that
AltCfgResp must contain AltConfigHdr. So we add ASSERT to assume
StringPtr is not NULL.
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Dandan Bi <dandan.bi@intel.com>
---
MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 71ea25bc19bf..2cad6d29f455 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -909,6 +909,7 @@ CompareAndMergeDefaultString (
// To find the <AltResp> with AltConfigHdr in AltCfgResp, ignore other <AltResp> which follow it.
//
StringPtr = StrStr (*AltCfgResp, AltConfigHdr);
+ ASSERT (StringPtr != NULL);
StringPtrNext = StrStr (StringPtr + 1, L"&GUID");
if (StringPtrNext != NULL) {
TempCharA = *StringPtrNext;
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 3/4] MdeModulePkg/EsrtDxe: Add check for EsrtRepository
2019-11-01 6:14 [PATCH v 3 0/4] MdeModulePkg: Add check and ASSERT for variables Zhang, Shenglei
2019-11-01 6:14 ` [PATCH v3 1/4] MdeModulePkg/EbcDebugger: Add check for Entry and RetEntry Zhang, Shenglei
2019-11-01 6:14 ` [PATCH v3 2/4] MdeModulePkg/HiiDatabaseDxe: ASSERT StringPtr Zhang, Shenglei
@ 2019-11-01 6:14 ` Zhang, Shenglei
2019-11-01 6:14 ` [PATCH v3 4/4] MdeModulePkg/SetupBrowserDxe: ASSERT GetBufferForValue(&Value) Zhang, Shenglei
3 siblings, 0 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2019-11-01 6:14 UTC (permalink / raw)
To: devel; +Cc: Hao A Wu, Liming Gao
EsrtRepository might be NULL. So return EFI_OUT_OF_RESOURCES
when it is NULL.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c b/MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c
index f48125382dbc..fff17b98fa3d 100644
--- a/MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c
+++ b/MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c
@@ -239,6 +239,11 @@ DeleteEsrtEntry(
goto EXIT;
}
+ if (EsrtRepository == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto EXIT;
+ }
+
if ((RepositorySize % sizeof(EFI_SYSTEM_RESOURCE_ENTRY)) != 0) {
DEBUG((EFI_D_ERROR, "Repository Corrupt. Need to rebuild Repository.\n"));
//
@@ -332,6 +337,11 @@ UpdateEsrtEntry(
&RepositorySize
);
+ if (EsrtRepository == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto EXIT;
+ }
+
if (!EFI_ERROR(Status)) {
//
// if exist, update Esrt cache repository
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 4/4] MdeModulePkg/SetupBrowserDxe: ASSERT GetBufferForValue(&Value)
2019-11-01 6:14 [PATCH v 3 0/4] MdeModulePkg: Add check and ASSERT for variables Zhang, Shenglei
` (2 preceding siblings ...)
2019-11-01 6:14 ` [PATCH v3 3/4] MdeModulePkg/EsrtDxe: Add check for EsrtRepository Zhang, Shenglei
@ 2019-11-01 6:14 ` Zhang, Shenglei
2019-11-05 2:24 ` [edk2-devel] " Dandan Bi
3 siblings, 1 reply; 6+ messages in thread
From: Zhang, Shenglei @ 2019-11-01 6:14 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu
Before called by GetBufferForValue(), Value has already been called
function IsTypeInBuffer to make sure the value must be buffer type.
So GetBufferForValue can not return NULL.
This commit adds ASSERT to assume (GetBufferForValue (&Value) is not
NULL.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
v3: Add ASSERT instead of using error handling.
MdeModulePkg/Universal/SetupBrowserDxe/Expression.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c b/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
index 7f4929c2fcd9..138912e00823 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
@@ -1281,7 +1281,10 @@ IfrToUint (
Result->Type = EFI_IFR_TYPE_UNDEFINED;
return EFI_SUCCESS;
}
+
+ ASSERT ((GetBufferForValue (&Value) != NULL);
Result->Value.u64 = *(UINT64*) GetBufferForValue (&Value);
+
if (Value.Type == EFI_IFR_TYPE_BUFFER) {
FreePool (Value.Buffer);
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [edk2-devel] [PATCH v3 4/4] MdeModulePkg/SetupBrowserDxe: ASSERT GetBufferForValue(&Value)
2019-11-01 6:14 ` [PATCH v3 4/4] MdeModulePkg/SetupBrowserDxe: ASSERT GetBufferForValue(&Value) Zhang, Shenglei
@ 2019-11-05 2:24 ` Dandan Bi
0 siblings, 0 replies; 6+ messages in thread
From: Dandan Bi @ 2019-11-05 2:24 UTC (permalink / raw)
To: devel@edk2.groups.io, Zhang, Shenglei; +Cc: Wang, Jian J, Wu, Hao A
Reviewed-by: Dandan Bi <dandan.bi@intel.com>
Thanks,
Dandan
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Zhang, Shenglei
> Sent: Friday, November 1, 2019 2:14 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [edk2-devel] [PATCH v3 4/4] MdeModulePkg/SetupBrowserDxe:
> ASSERT GetBufferForValue(&Value)
>
> Before called by GetBufferForValue(), Value has already been called function
> IsTypeInBuffer to make sure the value must be buffer type.
> So GetBufferForValue can not return NULL.
> This commit adds ASSERT to assume (GetBufferForValue (&Value) is not
> NULL.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
>
> v3: Add ASSERT instead of using error handling.
>
> MdeModulePkg/Universal/SetupBrowserDxe/Expression.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
> b/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
> index 7f4929c2fcd9..138912e00823 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
> @@ -1281,7 +1281,10 @@ IfrToUint (
> Result->Type = EFI_IFR_TYPE_UNDEFINED;
> return EFI_SUCCESS;
> }
> +
> + ASSERT ((GetBufferForValue (&Value) != NULL);
> Result->Value.u64 = *(UINT64*) GetBufferForValue (&Value);
> +
> if (Value.Type == EFI_IFR_TYPE_BUFFER) {
> FreePool (Value.Buffer);
> }
> --
> 2.18.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread