Hi, Andrew,

Thanks for your review. 

This new-added ASSERT() is for the internal function of this file, and external 
invocation has guaranteed the pointer is valid. So here ASSERT() is just to 
eliminate the possible warning information from some static-scanning tools. 


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Andrew Fish [mailto:[email protected]] 
Sent: Wednesday, December 03, 2014 3:52 PM
To: [email protected]
Cc: [email protected]
Subject: Re: edk2[16468] Code clean-up to eliminate potential "dereferenced 
pointer" warning.


> On Dec 2, 2014, at 11:40 PM, [email protected] wrote:
> 
> Revision: 16468
>          http://sourceforge.net/p/edk2/code/16468
> Author:   qlong
> Date:     2014-12-03 07:40:32 +0000 (Wed, 03 Dec 2014)
> Log Message:
> -----------
> Code clean-up to eliminate potential "dereferenced pointer" warning. 
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> 
> Signed-off-by: Qin Long <[email protected]>
> Reviewed-by: Guo Dong <[email protected]>
> Reviewed-by: Eric Dong <[email protected]>
> 
> Modified Paths:
> --------------
>    
> trunk/edk2/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerific
> ationLib.c
> 
> Modified: 
> trunk/edk2/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerific
> ationLib.c 
> ===================================================================
> --- 
> trunk/edk2/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>   2014-12-02 21:30:41 UTC (rev 16467)
> +++ 
> trunk/edk2/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>   2014-12-03 07:40:32 UTC (rev 16468)
> @@ -860,6 +860,7 @@
>   HashAlg  = HASHALG_MAX;
> 
>   ASSERT (RevocationTime != NULL);
> +  ASSERT (DbxList != NULL);
> 

This does not work if MDEPKG_NDEBUG is defined to fix a dereferenced pointer 
issue. 

Thanks,

Andrew Fish

>   while ((DbxSize > 0) && (SignatureListSize >= DbxList->SignatureListSize)) {
>     //
> @@ -1132,16 +1133,17 @@
>   //
>   DbtDataSize = 0;
>   Status   = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE2, 
> &gEfiImageSecurityDatabaseGuid, NULL, &DbtDataSize, NULL);
> -  if (Status == EFI_BUFFER_TOO_SMALL) {
> -    DbtData = (UINT8 *) AllocateZeroPool (DbtDataSize);
> -    if (DbtData == NULL) {
> -      goto Done;
> -    }
> -    Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE2, 
> &gEfiImageSecurityDatabaseGuid, NULL, &DbtDataSize, (VOID *) DbtData);
> -    if (EFI_ERROR (Status)) {
> -      goto Done;
> -    }
> +  if (Status != EFI_BUFFER_TOO_SMALL) {
> +    goto Done;
>   }
> +  DbtData = (UINT8 *) AllocateZeroPool (DbtDataSize);  if (DbtData == 
> + NULL) {
> +    goto Done;
> +  }
> +  Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE2, 
> + &gEfiImageSecurityDatabaseGuid, NULL, &DbtDataSize, (VOID *) DbtData);  if 
> (EFI_ERROR (Status)) {
> +    goto Done;
> +  }
> 
>   CertList = (EFI_SIGNATURE_LIST *) DbtData;
>   while ((DbtDataSize > 0) && (DbtDataSize >= 
> CertList->SignatureListSize)) { @@ -1229,14 +1231,15 @@
>   //
>   DataSize = 0;
>   Status   = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, 
> &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL);
> -  if (Status == EFI_BUFFER_TOO_SMALL) {
> -    Data = (UINT8 *) AllocateZeroPool (DataSize);
> -    if (Data == NULL) {
> -      return IsForbidden;
> -    }
> +  if (Status != EFI_BUFFER_TOO_SMALL) {
> +    return IsForbidden;
> +  }
> +  Data = (UINT8 *) AllocateZeroPool (DataSize);  if (Data == NULL) {
> +    return IsForbidden;
> +  }
> 
> -    Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, 
> &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, (VOID *) Data);
> -  }
> +  Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, 
> + &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, (VOID *) Data);
>   if (EFI_ERROR (Status)) {
>     return IsForbidden;
>   }
> @@ -1254,7 +1257,7 @@
>   //       UINT8  Certn[];
>   //
>   Pkcs7GetSigners (AuthData, AuthDataSize, &CertBuffer, &BufferLength, 
> &TrustedCert, &TrustedCertLength);
> -  if (BufferLength == 0) {
> +  if ((BufferLength == 0) || (CertBuffer == NULL)) {
>     IsForbidden = TRUE;
>     goto Done;
>   }
> 
> 
> ----------------------------------------------------------------------
> -------- Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT 
> Server from Actuate! Instantly Supercharge Your Business Reports and 
> Dashboards with Interactivity, Sharing, Native Excel Exports, App 
> Integration & more Get technology previously reserved for 
> billion-dollar corporations, FREE 
> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.
> clktrk _______________________________________________
> edk2-commits mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/edk2-commits


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! 
Instantly Supercharge Your Business Reports and Dashboards with Interactivity, 
Sharing, Native Excel Exports, App Integration & more Get technology previously 
reserved for billion-dollar corporations, FREE 
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits

Reply via email to