[PATCH] MdePkg DxeServicesLib: Handle potential NULL FvHandle

[PATCH] MdePkg DxeServicesLib: Handle potential NULL FvHandle

[Star Zeng]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=514

The FvHandle input to InternalGetSectionFromFv() may be NULL,
then ASSERT will appear. It is because the LoadedImage->DeviceHandle
returned from InternalImageHandleToFvHandle() may be NULL.
For example for DxeCore, there is LoadedImage protocol installed
for it, but the LoadedImage->DeviceHandle could not be initialized
before the FV2 (contain DxeCore) protocol is installed.

This patch is to update InternalGetSectionFromFv() to return
EFI_NOT_FOUND directly for NULL FvHandle.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdePkg/Library/DxeServicesLib/DxeServicesLib.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
index 2adf76fd8d22..1827c9216fbc 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
@@ -2,7 +2,7 @@
   MDE DXE Services Library provides functions that simplify the development of DXE Drivers.  
   These functions help access data from sections of FFS files or from file path.
 
-  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
   (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
@@ -62,6 +62,12 @@ InternalImageHandleToFvHandle (
 
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // The LoadedImage->DeviceHandle may be NULL.
+  // For example for DxeCore, there is LoadedImage protocol installed for it, but the
+  // LoadedImage->DeviceHandle could not be initialized before the FV2 (contain DxeCore)
+  // protocol is installed.
+  //
   return LoadedImage->DeviceHandle;
 
 }
@@ -84,7 +90,6 @@ InternalImageHandleToFvHandle (
   The data and size is returned by Buffer and Size. The caller is responsible to free the Buffer allocated 
   by this function. This function can be only called at TPL_NOTIFY and below.
   
-  If FvHandle is NULL, then ASSERT ();
   If NameGuid is NULL, then ASSERT();
   If Buffer is NULL, then ASSERT();
   If Size is NULL, then ASSERT().
@@ -128,7 +133,12 @@ InternalGetSectionFromFv (
   ASSERT (Buffer != NULL);
   ASSERT (Size != NULL);
   
-  ASSERT (FvHandle != NULL);
+  if (FvHandle == NULL) {
+    //
+    // Return EFI_NOT_FOUND directly for NULL FvHandle.
+    //
+    return EFI_NOT_FOUND;
+  }
 
   Status = gBS->HandleProtocol (
                   FvHandle,
-- 
2.7.0.windows.1

Re: [PATCH] MdePkg DxeServicesLib: Handle potential NULL FvHandle

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Zeng, Star
>Sent: Friday, May 05, 2017 4:21 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>;
>Kinney, Michael D <michael.d.kinney@intel.com>; Michael Turner
><Michael.Turner@microsoft.com>
>Subject: [PATCH] MdePkg DxeServicesLib: Handle potential NULL FvHandle
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=514
>
>The FvHandle input to InternalGetSectionFromFv() may be NULL,
>then ASSERT will appear. It is because the LoadedImage->DeviceHandle
>returned from InternalImageHandleToFvHandle() may be NULL.
>For example for DxeCore, there is LoadedImage protocol installed
>for it, but the LoadedImage->DeviceHandle could not be initialized
>before the FV2 (contain DxeCore) protocol is installed.
>
>This patch is to update InternalGetSectionFromFv() to return
>EFI_NOT_FOUND directly for NULL FvHandle.
>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Michael Kinney <michael.d.kinney@intel.com>
>Cc: Michael Turner <Michael.Turner@microsoft.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng <star.zeng@intel.com>
>---
> MdePkg/Library/DxeServicesLib/DxeServicesLib.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
>diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
>b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
>index 2adf76fd8d22..1827c9216fbc 100644
>--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
>+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
>@@ -2,7 +2,7 @@
>   MDE DXE Services Library provides functions that simplify the development
>of DXE Drivers.
>   These functions help access data from sections of FFS files or from file path.
>
>-  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
>+  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
>   (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
>   This program and the accompanying materials
>   are licensed and made available under the terms and conditions of the BSD
>License
>@@ -62,6 +62,12 @@ InternalImageHandleToFvHandle (
>
>   ASSERT_EFI_ERROR (Status);
>
>+  //
>+  // The LoadedImage->DeviceHandle may be NULL.
>+  // For example for DxeCore, there is LoadedImage protocol installed for it,
>but the
>+  // LoadedImage->DeviceHandle could not be initialized before the FV2
>(contain DxeCore)
>+  // protocol is installed.
>+  //
>   return LoadedImage->DeviceHandle;
>
> }
>@@ -84,7 +90,6 @@ InternalImageHandleToFvHandle (
>   The data and size is returned by Buffer and Size. The caller is responsible to
>free the Buffer allocated
>   by this function. This function can be only called at TPL_NOTIFY and below.
>
>-  If FvHandle is NULL, then ASSERT ();
>   If NameGuid is NULL, then ASSERT();
>   If Buffer is NULL, then ASSERT();
>   If Size is NULL, then ASSERT().
>@@ -128,7 +133,12 @@ InternalGetSectionFromFv (
>   ASSERT (Buffer != NULL);
>   ASSERT (Size != NULL);
>
>-  ASSERT (FvHandle != NULL);
>+  if (FvHandle == NULL) {
>+    //
>+    // Return EFI_NOT_FOUND directly for NULL FvHandle.
>+    //
>+    return EFI_NOT_FOUND;
>+  }
>
>   Status = gBS->HandleProtocol (
>                   FvHandle,
>--
>2.7.0.windows.1