public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 1/7] FmpDevicePkg/FmpDependencyLib: Correct ValidateDependency() documentation
       [not found] <20200805213944.1811-1-michael.kubacki@outlook.com>
@ 2020-08-05 21:39 ` Michael Kubacki
  2020-08-05 21:39 ` [PATCH v2 2/7] FmpDevicePkg/FmpDependencyLib: Fix "exression" typo Michael Kubacki
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 7+ messages in thread
From: Michael Kubacki @ 2020-08-05 21:39 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

From: Michael Kubacki <michael.kubacki@microsoft.com>

Modifies the return value documentation to state that the BOOLEAN
value indicates whether a given dependency expression is valid
not a capsule.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
---
 FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c | 5 +++--
 FmpDevicePkg/Include/Library/FmpDependencyLib.h          | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
index 91dc0b9abd33..28358069950a 100644
--- a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
+++ b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
@@ -1,6 +1,7 @@
 /** @file
   Supports Fmp Capsule Dependency Expression.
 
+  Copyright (c) Microsoft Corporation.<BR>
   Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -433,8 +434,8 @@ EvaluateDependency (
   @param[in]   MaxDepexSize   Max size of the dependency.
   @param[out]  DepexSize      Size of dependency.
 
-  @retval TRUE    The capsule is valid.
-  @retval FALSE   The capsule is invalid.
+  @retval TRUE    The dependency expression is valid.
+  @retval FALSE   The dependency expression is invalid.
 
 **/
 BOOLEAN
diff --git a/FmpDevicePkg/Include/Library/FmpDependencyLib.h b/FmpDevicePkg/Include/Library/FmpDependencyLib.h
index 1110eefa9a54..c732903425b4 100644
--- a/FmpDevicePkg/Include/Library/FmpDependencyLib.h
+++ b/FmpDevicePkg/Include/Library/FmpDependencyLib.h
@@ -2,6 +2,7 @@
   Fmp Capsule Dependency support functions for Firmware Management Protocol based
   firmware updates.
 
+  Copyright (c) Microsoft Corporation.<BR>
   Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -29,8 +30,8 @@ typedef struct {
   @param[in]   MaxDepexSize   Max size of the dependency.
   @param[out]  DepexSize      Size of dependency.
 
-  @retval TRUE    The capsule is valid.
-  @retval FALSE   The capsule is invalid.
+  @retval TRUE    The dependency expression is valid.
+  @retval FALSE   The dependency expression is invalid.
 
 **/
 BOOLEAN
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/7] FmpDevicePkg/FmpDependencyLib: Fix "exression" typo
       [not found] <20200805213944.1811-1-michael.kubacki@outlook.com>
  2020-08-05 21:39 ` [PATCH v2 1/7] FmpDevicePkg/FmpDependencyLib: Correct ValidateDependency() documentation Michael Kubacki
@ 2020-08-05 21:39 ` Michael Kubacki
  2020-08-05 21:39 ` [PATCH v2 3/7] FmpDevicePkg/FmpDependencyLib: Handle version string overflow Michael Kubacki
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 7+ messages in thread
From: Michael Kubacki @ 2020-08-05 21:39 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

From: Michael Kubacki <michael.kubacki@microsoft.com>

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
---
 FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
index 28358069950a..ba89eb22d9f0 100644
--- a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
+++ b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
@@ -421,7 +421,7 @@ EvaluateDependency (
     Iterator++;
   }
 
-  DEBUG ((DEBUG_ERROR, "EvaluateDependency: No EFI_FMP_DEP_END Opcode in exression!\n"));
+  DEBUG ((DEBUG_ERROR, "EvaluateDependency: No EFI_FMP_DEP_END Opcode in expression!\n"));
 
 Error:
   return FALSE;
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/7] FmpDevicePkg/FmpDependencyLib: Handle version string overflow
       [not found] <20200805213944.1811-1-michael.kubacki@outlook.com>
  2020-08-05 21:39 ` [PATCH v2 1/7] FmpDevicePkg/FmpDependencyLib: Correct ValidateDependency() documentation Michael Kubacki
  2020-08-05 21:39 ` [PATCH v2 2/7] FmpDevicePkg/FmpDependencyLib: Fix "exression" typo Michael Kubacki
@ 2020-08-05 21:39 ` Michael Kubacki
  2020-08-05 21:39 ` [PATCH v2 4/7] FmpDevicePkg/FmpDependencyCheckLib: Return unsatisfied on handle failure Michael Kubacki
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 7+ messages in thread
From: Michael Kubacki @ 2020-08-05 21:39 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

From: Michael Kubacki <michael.kubacki@microsoft.com>

This change recognizes the condition of the DEPEX version string
extending beyond the end of the dependency expression as an error.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
---

Notes:
    This is particularly helpful for the user to isolate the issue
    when stepping through the control flow as this case will be the
    last executed before jumping to the Error label to return from
    the function.

 FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
index ba89eb22d9f0..5ef25d2415cf 100644
--- a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
+++ b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
@@ -286,6 +286,7 @@ EvaluateDependency (
       Iterator += AsciiStrnLenS ((CHAR8 *) Iterator, DependenciesSize - (Iterator - Dependencies->Dependencies));
       if (Iterator == (UINT8 *) Dependencies->Dependencies + DependenciesSize) {
         DEBUG ((DEBUG_ERROR, "EvaluateDependency: STRING extends beyond end of dependency expression!\n"));
+        goto Error;
       }
       break;
     case EFI_FMP_DEP_AND:
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 4/7] FmpDevicePkg/FmpDependencyCheckLib: Return unsatisfied on handle failure
       [not found] <20200805213944.1811-1-michael.kubacki@outlook.com>
                   ` (2 preceding siblings ...)
  2020-08-05 21:39 ` [PATCH v2 3/7] FmpDevicePkg/FmpDependencyLib: Handle version string overflow Michael Kubacki
@ 2020-08-05 21:39 ` Michael Kubacki
  2020-08-05 21:39 ` [PATCH v2 5/7] FmpDevicePkg/FmpDxe: Better warn of potential ImageTypeId misconfig Michael Kubacki
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 7+ messages in thread
From: Michael Kubacki @ 2020-08-05 21:39 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

From: Michael Kubacki <michael.kubacki@microsoft.com>

CheckFmpDependency () will currently return that dependencies are
satisfied if the initial call in the function to locate handles
that have gEfiFirmwareManagementProtocolGuid installed fails.

This change updates the error handling to return FALSE (dependencies
are not satisfied) if this handle search fails.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
---
 FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c b/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
index 5e0241b25957..02ed600e0e95 100644
--- a/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
+++ b/FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c
@@ -2,6 +2,7 @@
   Provides FMP capsule dependency check services when updating the firmware
   image of a FMP device.
 
+  Copyright (c) Microsoft Corporation.<BR>
   Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -75,6 +76,7 @@ CheckFmpDependency (
                 );
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "CheckFmpDependency: Get Firmware Management Protocol failed. (%r)", Status));
+    IsSatisfied = FALSE;
     goto cleanup;
   }
 
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 5/7] FmpDevicePkg/FmpDxe: Better warn of potential ImageTypeId misconfig
       [not found] <20200805213944.1811-1-michael.kubacki@outlook.com>
                   ` (3 preceding siblings ...)
  2020-08-05 21:39 ` [PATCH v2 4/7] FmpDevicePkg/FmpDependencyCheckLib: Return unsatisfied on handle failure Michael Kubacki
@ 2020-08-05 21:39 ` Michael Kubacki
  2020-08-05 21:39 ` [PATCH v2 6/7] FmpDevicePkg/FmpDxe: Indicate ESRT GUID on invalid ImageIdName Michael Kubacki
  2020-08-05 21:39 ` [PATCH v2 7/7] FmpDevicePkg/FmpDxe: Improve function parameter validation Michael Kubacki
  6 siblings, 0 replies; 7+ messages in thread
From: Michael Kubacki @ 2020-08-05 21:39 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

From: Michael Kubacki <michael.kubacki@microsoft.com>

A user may fall through to the case they depend on the
PcdFmpDeviceImageTypeIdGuid value to get the ImageTypeId GUID
value. The default PCD value is 0 (NULL) so the code would
further fall back on the gEfiCallerIdGuid value.

This change modifies the print error level for the message that
indicates this occurred to DEBUG_WARN from DEBUG_INFO to better
warn the user that this occurred.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
---

Notes:
    The PCD documentation indicates it is valid behavior for the user
    to fall back to gEfiCallerIdGuid. Is that really expected?
    
    Would an ASSERT be appropriate?

 FmpDevicePkg/FmpDxe/FmpDxe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 58841774fee0..14994ce4ee0e 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -3,7 +3,7 @@
   image stored in a firmware device with platform and firmware device specific
   information provided through PCDs and libraries.
 
-  Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR>
+  Copyright (c) Microsoft Corporation.<BR>
   Copyright (c) 2018 - 2020, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -179,7 +179,7 @@ GetImageTypeIdGuid (
     if (ImageTypeIdGuidSize == sizeof (EFI_GUID)) {
       FmpDeviceLibGuid = (EFI_GUID *)PcdGetPtr (PcdFmpDeviceImageTypeIdGuid);
     } else {
-      DEBUG ((DEBUG_INFO, "FmpDxe(%s): Fall back to ImageTypeIdGuid of gEfiCallerIdGuid\n", mImageIdName));
+      DEBUG ((DEBUG_WARN, "FmpDxe(%s): Fall back to ImageTypeIdGuid of gEfiCallerIdGuid\n", mImageIdName));
       FmpDeviceLibGuid = &gEfiCallerIdGuid;
     }
   }
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 6/7] FmpDevicePkg/FmpDxe: Indicate ESRT GUID on invalid ImageIdName
       [not found] <20200805213944.1811-1-michael.kubacki@outlook.com>
                   ` (4 preceding siblings ...)
  2020-08-05 21:39 ` [PATCH v2 5/7] FmpDevicePkg/FmpDxe: Better warn of potential ImageTypeId misconfig Michael Kubacki
@ 2020-08-05 21:39 ` Michael Kubacki
  2020-08-05 21:39 ` [PATCH v2 7/7] FmpDevicePkg/FmpDxe: Improve function parameter validation Michael Kubacki
  6 siblings, 0 replies; 7+ messages in thread
From: Michael Kubacki @ 2020-08-05 21:39 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

From: Michael Kubacki <michael.kubacki@microsoft.com>

Updates the debug error message to include the GUID of the FMP
instance that encountered the issue to help the user better
isolate the problem.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index 14994ce4ee0e..a3e342591936 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -1679,7 +1679,7 @@ FmpDxeEntryPoint (
     //
     // PcdFmpDeviceImageIdName must be set to a non-empty Unicode string
     //
-    DEBUG ((DEBUG_ERROR, "FmpDxe: PcdFmpDeviceImageIdName is an empty string.\n"));
+    DEBUG ((DEBUG_ERROR, "FmpDxe(%g): PcdFmpDeviceImageIdName is an empty string.\n", &gEfiCallerIdGuid));
     ASSERT (FALSE);
     return EFI_UNSUPPORTED;
   }
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 7/7] FmpDevicePkg/FmpDxe: Improve function parameter validation
       [not found] <20200805213944.1811-1-michael.kubacki@outlook.com>
                   ` (5 preceding siblings ...)
  2020-08-05 21:39 ` [PATCH v2 6/7] FmpDevicePkg/FmpDxe: Indicate ESRT GUID on invalid ImageIdName Michael Kubacki
@ 2020-08-05 21:39 ` Michael Kubacki
  6 siblings, 0 replies; 7+ messages in thread
From: Michael Kubacki @ 2020-08-05 21:39 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Michael D Kinney, Guomin Jiang, Wei6 Xu

From: Michael Kubacki <michael.kubacki@microsoft.com>

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

Makes some minor improvements to function parameter validation
in FmpDxe, in particular to externally exposed functions such
as those that back EFI_FIRMWARE_MANAGEMENT_PROTOCOL.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Wei6 Xu <wei6.xu@intel.com>
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 59 +++++++++++++++++---
 FmpDevicePkg/FmpDxe/FmpDxe.h | 10 ++--
 2 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index a3e342591936..9fa63cff4aa2 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -278,6 +278,11 @@ PopulateDescriptor (
   EFI_STATUS  Status;
   UINT32      DependenciesSize;
 
+  if (Private == NULL) {
+    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): PopulateDescriptor() - Private is NULL.\n", mImageIdName));
+    return;
+  }
+
   if (Private->DescriptorPopulated) {
     return;
   }
@@ -429,7 +434,7 @@ PopulateDescriptor (
   @retval EFI_SUCCESS                The device was successfully updated with the new image.
   @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too small. The current buffer size
                                      needed to hold the image(s) information is returned in ImageInfoSize.
-  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
+  @retval EFI_INVALID_PARAMETER      A required pointer is NULL.
   @retval EFI_DEVICE_ERROR           Valid information could not be returned. Possible corrupted image.
 
 **/
@@ -451,6 +456,12 @@ GetTheImageInfo (
 
   Status = EFI_SUCCESS;
 
+  if (This == NULL) {
+    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImageInfo() - This is NULL.\n", mImageIdName));
+    Status = EFI_INVALID_PARAMETER;
+    goto cleanup;
+  }
+
   //
   // Retrieve the private context structure
   //
@@ -536,7 +547,7 @@ GetTheImageInfo (
   @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too small to hold the
                                  image. The current buffer size needed to hold the image is returned
                                  in ImageSize.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  A required pointer is NULL or ImageIndex is invalid.
   @retval EFI_NOT_FOUND          The current image is not copied to the buffer.
   @retval EFI_UNSUPPORTED        The operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
@@ -561,6 +572,12 @@ GetTheImage (
 
   Status = EFI_SUCCESS;
 
+  if (This == NULL) {
+    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImage() - This is NULL.\n", mImageIdName));
+    Status = EFI_INVALID_PARAMETER;
+    goto cleanup;
+  }
+
   //
   // Retrieve the private context structure
   //
@@ -615,7 +632,8 @@ GetTheImage (
   @param[in]   Image                 Pointer to the image.
   @param[in]   ImageSize             Size of the image.
   @param[in]   AdditionalHeaderSize  Size of any headers that cannot be calculated by this function.
-  @param[out]  PayloadSize
+  @param[out]  PayloadSize           An optional pointer to a UINTN that holds the size of the payload
+                                     (image size minus headers)
 
   @retval  !NULL  Valid pointer to the header.
   @retval  NULL   Structure is bad and pointer cannot be found.
@@ -626,7 +644,7 @@ GetFmpHeader (
   IN  CONST EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
   IN  CONST UINTN                              ImageSize,
   IN  CONST UINTN                              AdditionalHeaderSize,
-  OUT UINTN                                    *PayloadSize
+  OUT UINTN                                    *PayloadSize OPTIONAL
   )
 {
   //
@@ -640,7 +658,10 @@ GetFmpHeader (
     return NULL;
   }
 
-  *PayloadSize = ImageSize - (sizeof (Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength + AdditionalHeaderSize);
+  if (PayloadSize != NULL) {
+    *PayloadSize = ImageSize - (sizeof (Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength + AdditionalHeaderSize);
+  }
+
   return (VOID *)((UINT8 *)Image + sizeof (Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength  + AdditionalHeaderSize);
 }
 
@@ -663,6 +684,11 @@ GetAllHeaderSize (
 {
   UINT32  CalculatedSize;
 
+  if (Image == NULL) {
+    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetAllHeaderSize() - Image is NULL.\n", mImageIdName));
+    return 0;
+  }
+
   CalculatedSize = sizeof (Image->MonotonicCount) +
                    AdditionalHeaderSize +
                    Image->AuthInfo.Hdr.dwLength;
@@ -698,7 +724,7 @@ GetAllHeaderSize (
 
   @retval EFI_SUCCESS            The image was successfully checked.
   @retval EFI_ABORTED            The operation is aborted.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  A required pointer is NULL or ImageIndex is invalid.
   @retval EFI_UNSUPPORTED        The operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
 
@@ -743,6 +769,12 @@ CheckTheImage (
     return EFI_UNSUPPORTED;
   }
 
+  if (This == NULL) {
+    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - This is NULL.\n", mImageIdName));
+    Status = EFI_INVALID_PARAMETER;
+    goto cleanup;
+  }
+
   //
   // Retrieve the private context structure
   //
@@ -851,7 +883,7 @@ CheckTheImage (
   if (ImageIndex != 1) {
     DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - Image Index Invalid.\n", mImageIdName));
     *ImageUpdatable = IMAGE_UPDATABLE_INVALID_TYPE;
-    Status = EFI_SUCCESS;
+    Status = EFI_INVALID_PARAMETER;
     goto cleanup;
   }
 
@@ -978,7 +1010,7 @@ CheckTheImage (
 
   @retval EFI_SUCCESS            The device was successfully updated with the new image.
   @retval EFI_ABORTED            The operation is aborted.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  A required pointer is NULL or ImageIndex is invalid.
   @retval EFI_UNSUPPORTED        The operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
 
@@ -1026,6 +1058,12 @@ SetTheImage (
     return EFI_UNSUPPORTED;
   }
 
+  if (This == NULL) {
+    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - This is NULL.\n", mImageIdName));
+    Status = EFI_INVALID_PARAMETER;
+    goto cleanup;
+  }
+
   //
   // Retrieve the private context structure
   //
@@ -1382,6 +1420,11 @@ FmpDxeLockEventNotify (
   EFI_STATUS                        Status;
   FIRMWARE_MANAGEMENT_PRIVATE_DATA  *Private;
 
+  if (Context == NULL) {
+    ASSERT (Context != NULL);
+    return;
+  }
+
   Private = (FIRMWARE_MANAGEMENT_PRIVATE_DATA *)Context;
 
   if (!Private->FmpDeviceLocked) {
diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.h b/FmpDevicePkg/FmpDxe/FmpDxe.h
index 30754dea495e..7cb273a41b78 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.h
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.h
@@ -3,7 +3,7 @@
   image stored in a firmware device with platform and firmware device specific
   information provided through PCDs and libraries.
 
-  Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR>
+  Copyright (c) Microsoft Corporation.<BR>
   Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -132,7 +132,7 @@ DetectTestKey (
   @retval EFI_SUCCESS                The device was successfully updated with the new image.
   @retval EFI_BUFFER_TOO_SMALL       The ImageInfo buffer was too small. The current buffer size
                                      needed to hold the image(s) information is returned in ImageInfoSize.
-  @retval EFI_INVALID_PARAMETER      ImageInfoSize is NULL.
+  @retval EFI_INVALID_PARAMETER      A required pointer is NULL.
   @retval EFI_DEVICE_ERROR           Valid information could not be returned. Possible corrupted image.
 
 **/
@@ -166,7 +166,7 @@ GetTheImageInfo (
   @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too small to hold the
                                  image. The current buffer size needed to hold the image is returned
                                  in ImageSize.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  A required pointer is NULL or ImageIndex is invalid.
   @retval EFI_NOT_FOUND          The current image is not copied to the buffer.
   @retval EFI_UNSUPPORTED        The operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
@@ -198,7 +198,7 @@ GetTheImage (
 
   @retval EFI_SUCCESS            The image was successfully checked.
   @retval EFI_ABORTED            The operation is aborted.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  A required pointer is NULL or ImageIndex is invalid.
   @retval EFI_UNSUPPORTED        The operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
 
@@ -254,7 +254,7 @@ CheckTheImage (
 
   @retval EFI_SUCCESS            The device was successfully updated with the new image.
   @retval EFI_ABORTED            The operation is aborted.
-  @retval EFI_INVALID_PARAMETER  The Image was NULL.
+  @retval EFI_INVALID_PARAMETER  A required pointer is NULL or ImageIndex is invalid.
   @retval EFI_UNSUPPORTED        The operation is not supported.
   @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure.
 
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-08-05 21:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20200805213944.1811-1-michael.kubacki@outlook.com>
2020-08-05 21:39 ` [PATCH v2 1/7] FmpDevicePkg/FmpDependencyLib: Correct ValidateDependency() documentation Michael Kubacki
2020-08-05 21:39 ` [PATCH v2 2/7] FmpDevicePkg/FmpDependencyLib: Fix "exression" typo Michael Kubacki
2020-08-05 21:39 ` [PATCH v2 3/7] FmpDevicePkg/FmpDependencyLib: Handle version string overflow Michael Kubacki
2020-08-05 21:39 ` [PATCH v2 4/7] FmpDevicePkg/FmpDependencyCheckLib: Return unsatisfied on handle failure Michael Kubacki
2020-08-05 21:39 ` [PATCH v2 5/7] FmpDevicePkg/FmpDxe: Better warn of potential ImageTypeId misconfig Michael Kubacki
2020-08-05 21:39 ` [PATCH v2 6/7] FmpDevicePkg/FmpDxe: Indicate ESRT GUID on invalid ImageIdName Michael Kubacki
2020-08-05 21:39 ` [PATCH v2 7/7] FmpDevicePkg/FmpDxe: Improve function parameter validation Michael Kubacki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox