public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] SignedCapsulePkg: Address NULL pointer dereference case.
@ 2020-07-13  2:37 Vin Xue
  0 siblings, 0 replies; 4+ messages in thread
From: Vin Xue @ 2020-07-13  2:37 UTC (permalink / raw)
  To: devel

Original code GetFmpImageDescriptors for OriginalFmpImageInfoBuf
pointer, if failed, return a NULL pointer. The OriginalFmpImageInfoBuf
should not be NULL and the NULL pointer dereference case
should be false positive.

Signed-off-by: Vin Xue <vinxue@outlook.com>
---
 .../SystemFirmwareUpdateDxe.c                 | 39 ++++++++++---------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
index bdb70bdb32..ea795cd7db 100644
--- a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
+++ b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
@@ -681,32 +681,35 @@ FindMatchingFmpHandles (
     //
     // Loop through the set of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
     //
-    FmpImageInfoBuf = OriginalFmpImageInfoBuf;
     MatchFound = FALSE;
-    for (Index2 = 0; Index2 < FmpImageInfoCount; Index2++) {
-      for (Index3 = 0; Index3 < mSystemFmpPrivate->DescriptorCount; Index3++) {
-        MatchFound = CompareGuid (
-                       &FmpImageInfoBuf->ImageTypeId,
-                       &mSystemFmpPrivate->ImageDescriptor[Index3].ImageTypeId
-                       );
+    if (OriginalFmpImageInfoBuf != NULL) {
+      FmpImageInfoBuf = OriginalFmpImageInfoBuf;
+
+      for (Index2 = 0; Index2 < FmpImageInfoCount; Index2++) {
+        for (Index3 = 0; Index3 < mSystemFmpPrivate->DescriptorCount; Index3++) {
+          MatchFound = CompareGuid (
+                        &FmpImageInfoBuf->ImageTypeId,
+                        &mSystemFmpPrivate->ImageDescriptor[Index3].ImageTypeId
+                        );
+          if (MatchFound) {
+            break;
+          }
+        }
         if (MatchFound) {
           break;
         }
+        //
+        // Increment the buffer pointer ahead by the size of the descriptor
+        //
+        FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 *)FmpImageInfoBuf) + DescriptorSize);
       }
       if (MatchFound) {
-        break;
+        HandleBuffer[*HandleCount] = HandleBuffer[Index];
+        (*HandleCount)++;
       }
-      //
-      // Increment the buffer pointer ahead by the size of the descriptor
-      //
-      FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 *)FmpImageInfoBuf) + DescriptorSize);
-    }
-    if (MatchFound) {
-      HandleBuffer[*HandleCount] = HandleBuffer[Index];
-      (*HandleCount)++;
-    }
 
-    FreePool (OriginalFmpImageInfoBuf);
+      FreePool (OriginalFmpImageInfoBuf);
+    }
   }
 
   if ((*HandleCount) == 0) {
-- 
2.27.0.windows.1


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

* [PATCH] SignedCapsulePkg: Address NULL pointer dereference case.
@ 2020-07-14  2:07 Vin Xue
  0 siblings, 0 replies; 4+ messages in thread
From: Vin Xue @ 2020-07-14  2:07 UTC (permalink / raw)
  To: devel; +Cc: Vin Xue

Original code GetFmpImageDescriptors for OriginalFmpImageInfoBuf
pointer, if failed, return a NULL pointer. The OriginalFmpImageInfoBuf
should not be NULL and the NULL pointer dereference case
should be false positive.

Signed-off-by: Vin Xue <vinxue@outlook.com>
---
 .../SystemFirmwareUpdateDxe.c                 | 39 ++++++++++---------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
index bdb70bdb32..ea795cd7db 100644
--- a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
+++ b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
@@ -681,32 +681,35 @@ FindMatchingFmpHandles (
     //
     // Loop through the set of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
     //
-    FmpImageInfoBuf = OriginalFmpImageInfoBuf;
     MatchFound = FALSE;
-    for (Index2 = 0; Index2 < FmpImageInfoCount; Index2++) {
-      for (Index3 = 0; Index3 < mSystemFmpPrivate->DescriptorCount; Index3++) {
-        MatchFound = CompareGuid (
-                       &FmpImageInfoBuf->ImageTypeId,
-                       &mSystemFmpPrivate->ImageDescriptor[Index3].ImageTypeId
-                       );
+    if (OriginalFmpImageInfoBuf != NULL) {
+      FmpImageInfoBuf = OriginalFmpImageInfoBuf;
+
+      for (Index2 = 0; Index2 < FmpImageInfoCount; Index2++) {
+        for (Index3 = 0; Index3 < mSystemFmpPrivate->DescriptorCount; Index3++) {
+          MatchFound = CompareGuid (
+                        &FmpImageInfoBuf->ImageTypeId,
+                        &mSystemFmpPrivate->ImageDescriptor[Index3].ImageTypeId
+                        );
+          if (MatchFound) {
+            break;
+          }
+        }
         if (MatchFound) {
           break;
         }
+        //
+        // Increment the buffer pointer ahead by the size of the descriptor
+        //
+        FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 *)FmpImageInfoBuf) + DescriptorSize);
       }
       if (MatchFound) {
-        break;
+        HandleBuffer[*HandleCount] = HandleBuffer[Index];
+        (*HandleCount)++;
       }
-      //
-      // Increment the buffer pointer ahead by the size of the descriptor
-      //
-      FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 *)FmpImageInfoBuf) + DescriptorSize);
-    }
-    if (MatchFound) {
-      HandleBuffer[*HandleCount] = HandleBuffer[Index];
-      (*HandleCount)++;
-    }
 
-    FreePool (OriginalFmpImageInfoBuf);
+      FreePool (OriginalFmpImageInfoBuf);
+    }
   }
 
   if ((*HandleCount) == 0) {
-- 
2.27.0.windows.1


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

* [PATCH] SignedCapsulePkg: Address NULL pointer dereference case.
@ 2020-07-14  2:09 Vin Xue
  2020-07-16  8:31 ` Yao, Jiewen
  0 siblings, 1 reply; 4+ messages in thread
From: Vin Xue @ 2020-07-14  2:09 UTC (permalink / raw)
  To: devel; +Cc: Vin Xue, Jiewen Yao, Chao Zhang

Original code GetFmpImageDescriptors for OriginalFmpImageInfoBuf
pointer, if failed, return a NULL pointer. The OriginalFmpImageInfoBuf
should not be NULL and the NULL pointer dereference case
should be false positive.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Vin Xue <vinxue@outlook.com>
---
 .../SystemFirmwareUpdateDxe.c                 | 39 ++++++++++---------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
index bdb70bdb32..ea795cd7db 100644
--- a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
+++ b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
@@ -681,32 +681,35 @@ FindMatchingFmpHandles (
     //
     // Loop through the set of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
     //
-    FmpImageInfoBuf = OriginalFmpImageInfoBuf;
     MatchFound = FALSE;
-    for (Index2 = 0; Index2 < FmpImageInfoCount; Index2++) {
-      for (Index3 = 0; Index3 < mSystemFmpPrivate->DescriptorCount; Index3++) {
-        MatchFound = CompareGuid (
-                       &FmpImageInfoBuf->ImageTypeId,
-                       &mSystemFmpPrivate->ImageDescriptor[Index3].ImageTypeId
-                       );
+    if (OriginalFmpImageInfoBuf != NULL) {
+      FmpImageInfoBuf = OriginalFmpImageInfoBuf;
+
+      for (Index2 = 0; Index2 < FmpImageInfoCount; Index2++) {
+        for (Index3 = 0; Index3 < mSystemFmpPrivate->DescriptorCount; Index3++) {
+          MatchFound = CompareGuid (
+                        &FmpImageInfoBuf->ImageTypeId,
+                        &mSystemFmpPrivate->ImageDescriptor[Index3].ImageTypeId
+                        );
+          if (MatchFound) {
+            break;
+          }
+        }
         if (MatchFound) {
           break;
         }
+        //
+        // Increment the buffer pointer ahead by the size of the descriptor
+        //
+        FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 *)FmpImageInfoBuf) + DescriptorSize);
       }
       if (MatchFound) {
-        break;
+        HandleBuffer[*HandleCount] = HandleBuffer[Index];
+        (*HandleCount)++;
       }
-      //
-      // Increment the buffer pointer ahead by the size of the descriptor
-      //
-      FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 *)FmpImageInfoBuf) + DescriptorSize);
-    }
-    if (MatchFound) {
-      HandleBuffer[*HandleCount] = HandleBuffer[Index];
-      (*HandleCount)++;
-    }
 
-    FreePool (OriginalFmpImageInfoBuf);
+      FreePool (OriginalFmpImageInfoBuf);
+    }
   }
 
   if ((*HandleCount) == 0) {
-- 
2.27.0.windows.1


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

* Re: [PATCH] SignedCapsulePkg: Address NULL pointer dereference case.
  2020-07-14  2:09 [PATCH] SignedCapsulePkg: Address NULL pointer dereference case Vin Xue
@ 2020-07-16  8:31 ` Yao, Jiewen
  0 siblings, 0 replies; 4+ messages in thread
From: Yao, Jiewen @ 2020-07-16  8:31 UTC (permalink / raw)
  To: Vin Xue, devel@edk2.groups.io; +Cc: Zhang, Chao B

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Vin Xue <vinxue@outlook.com>
> Sent: Tuesday, July 14, 2020 10:10 AM
> To: devel@edk2.groups.io
> Cc: Vin Xue <vinxue@outlook.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH] SignedCapsulePkg: Address NULL pointer dereference case.
> 
> Original code GetFmpImageDescriptors for OriginalFmpImageInfoBuf
> pointer, if failed, return a NULL pointer. The OriginalFmpImageInfoBuf
> should not be NULL and the NULL pointer dereference case
> should be false positive.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Signed-off-by: Vin Xue <vinxue@outlook.com>
> ---
>  .../SystemFirmwareUpdateDxe.c                 | 39 ++++++++++---------
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git
> a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdate
> Dxe.c
> b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdate
> Dxe.c
> index bdb70bdb32..ea795cd7db 100644
> ---
> a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdate
> Dxe.c
> +++
> b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdate
> Dxe.c
> @@ -681,32 +681,35 @@ FindMatchingFmpHandles (
>      //
> 
>      // Loop through the set of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> 
>      //
> 
> -    FmpImageInfoBuf = OriginalFmpImageInfoBuf;
> 
>      MatchFound = FALSE;
> 
> -    for (Index2 = 0; Index2 < FmpImageInfoCount; Index2++) {
> 
> -      for (Index3 = 0; Index3 < mSystemFmpPrivate->DescriptorCount; Index3++) {
> 
> -        MatchFound = CompareGuid (
> 
> -                       &FmpImageInfoBuf->ImageTypeId,
> 
> -                       &mSystemFmpPrivate->ImageDescriptor[Index3].ImageTypeId
> 
> -                       );
> 
> +    if (OriginalFmpImageInfoBuf != NULL) {
> 
> +      FmpImageInfoBuf = OriginalFmpImageInfoBuf;
> 
> +
> 
> +      for (Index2 = 0; Index2 < FmpImageInfoCount; Index2++) {
> 
> +        for (Index3 = 0; Index3 < mSystemFmpPrivate->DescriptorCount; Index3++)
> {
> 
> +          MatchFound = CompareGuid (
> 
> +                        &FmpImageInfoBuf->ImageTypeId,
> 
> +                        &mSystemFmpPrivate->ImageDescriptor[Index3].ImageTypeId
> 
> +                        );
> 
> +          if (MatchFound) {
> 
> +            break;
> 
> +          }
> 
> +        }
> 
>          if (MatchFound) {
> 
>            break;
> 
>          }
> 
> +        //
> 
> +        // Increment the buffer pointer ahead by the size of the descriptor
> 
> +        //
> 
> +        FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8
> *)FmpImageInfoBuf) + DescriptorSize);
> 
>        }
> 
>        if (MatchFound) {
> 
> -        break;
> 
> +        HandleBuffer[*HandleCount] = HandleBuffer[Index];
> 
> +        (*HandleCount)++;
> 
>        }
> 
> -      //
> 
> -      // Increment the buffer pointer ahead by the size of the descriptor
> 
> -      //
> 
> -      FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8
> *)FmpImageInfoBuf) + DescriptorSize);
> 
> -    }
> 
> -    if (MatchFound) {
> 
> -      HandleBuffer[*HandleCount] = HandleBuffer[Index];
> 
> -      (*HandleCount)++;
> 
> -    }
> 
> 
> 
> -    FreePool (OriginalFmpImageInfoBuf);
> 
> +      FreePool (OriginalFmpImageInfoBuf);
> 
> +    }
> 
>    }
> 
> 
> 
>    if ((*HandleCount) == 0) {
> 
> --
> 2.27.0.windows.1


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

end of thread, other threads:[~2020-07-16  8:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-14  2:09 [PATCH] SignedCapsulePkg: Address NULL pointer dereference case Vin Xue
2020-07-16  8:31 ` Yao, Jiewen
  -- strict thread matches above, loose matches on Subject: below --
2020-07-14  2:07 Vin Xue
2020-07-13  2:37 Vin Xue

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