public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/3] Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
@ 2019-02-26 16:04 Jian J Wang
  2019-02-26 16:04 ` [PATCH 1/3] MdeModulePkg/PeiCore: " Jian J Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jian J Wang @ 2019-02-26 16:04 UTC (permalink / raw)
  To: edk2-devel

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

To follow PI spec, ensure FfsFileHeader 8 bytes aligned.

Current code only handles (FwVolHeader->ExtHeaderOffset != 0) path,
update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path.

Star Zeng (3):
  MdeModulePkg/PeiCore: Ensure FfsFileHeader 8 bytes aligned
  MdeModulePkg/DxeCore: Ensure FfsFileHeader 8 bytes aligned
  IntelFrameworkModulePkg/FwVolDxe: Ensure FfsFileHeader 8 bytes aligned

 .../Universal/FirmwareVolume/FwVolDxe/FwVol.c |  4 +-
 MdeModulePkg/Core/Dxe/FwVol/FwVol.c           | 65 ++++---------------
 MdeModulePkg/Core/Pei/FwVol/FwVol.c           |  4 +-
 3 files changed, 18 insertions(+), 55 deletions(-)

-- 
2.17.1.windows.2



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

* [PATCH 1/3] MdeModulePkg/PeiCore: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
  2019-02-26 16:04 [PATCH 0/3] Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630] Jian J Wang
@ 2019-02-26 16:04 ` Jian J Wang
  2019-02-27  6:56   ` Wang, Jian J
  2019-02-26 16:04 ` [PATCH 2/3] MdeModulePkg/DxeCore: " Jian J Wang
  2019-02-26 16:04 ` [PATCH 3/3] IntelFrameworkModulePkg/FwVolDxe: " Jian J Wang
  2 siblings, 1 reply; 7+ messages in thread
From: Jian J Wang @ 2019-02-26 16:04 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Liming Gao, Hao Wu

From: Star Zeng <star.zeng@intel.com>

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

To follow PI spec, ensure FfsFileHeader 8 bytes aligned.

Current code only handles (FwVolHeader->ExtHeaderOffset != 0) path,
update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Pei/FwVol/FwVol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
index 0a67b96bf1..56440eacf0 100644
--- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
+++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
@@ -2,7 +2,7 @@
   Pei Core Firmware File System service routines.
 
 Copyright (c) 2015 HP Development Company, L.P.
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -316,10 +316,10 @@ FindFileEx (
       //
       FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) ((UINT8 *) FwVolHeader + FwVolHeader->ExtHeaderOffset);
       FfsFileHeader = (EFI_FFS_FILE_HEADER *) ((UINT8 *) FwVolExtHeader + FwVolExtHeader->ExtHeaderSize);
-      FfsFileHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsFileHeader, 8);
     } else {
       FfsFileHeader = (EFI_FFS_FILE_HEADER *)((UINT8 *) FwVolHeader + FwVolHeader->HeaderLength);
     }
+    FfsFileHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsFileHeader, 8);
   } else {
     if (IS_FFS_FILE2 (*FileHeader)) {
       if (!IsFfs3Fv) {
-- 
2.17.1.windows.2



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

* [PATCH 2/3] MdeModulePkg/DxeCore: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
  2019-02-26 16:04 [PATCH 0/3] Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630] Jian J Wang
  2019-02-26 16:04 ` [PATCH 1/3] MdeModulePkg/PeiCore: " Jian J Wang
@ 2019-02-26 16:04 ` Jian J Wang
  2019-02-27  6:55   ` Wang, Jian J
  2019-02-26 16:04 ` [PATCH 3/3] IntelFrameworkModulePkg/FwVolDxe: " Jian J Wang
  2 siblings, 1 reply; 7+ messages in thread
From: Jian J Wang @ 2019-02-26 16:04 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Liming Gao, Hao Wu

From: Star Zeng <star.zeng@intel.com>

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

To follow PI spec, ensure FfsFileHeader 8 bytes aligned.

For the integrity of FV(especially non-MemoryMapped FV) layout,
let CachedFv point to FV beginning, but not (FV + FV header).

And current code only handles (FwVolHeader->ExtHeaderOffset != 0) path,
update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Dxe/FwVol/FwVol.c | 65 +++++++----------------------
 1 file changed, 14 insertions(+), 51 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
index 93ddcc3591..28fce46a95 100644
--- a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
+++ b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
@@ -3,7 +3,7 @@
   Layers on top of Firmware Block protocol to produce a file abstraction
   of FV based files.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -329,8 +329,6 @@ FvCheck (
   FFS_FILE_LIST_ENTRY                   *FfsFileEntry;
   EFI_FFS_FILE_HEADER                   *FfsHeader;
   UINT8                                 *CacheLocation;
-  UINTN                                 LbaOffset;
-  UINTN                                 HeaderSize;
   UINTN                                 Index;
   EFI_LBA                               LbaIndex;
   UINTN                                 Size;
@@ -353,11 +351,7 @@ FvCheck (
     return Status;
   }
 
-  //
-  // Size is the size of the FV minus the head. We have already allocated
-  // the header to check to make sure the volume is valid
-  //
-  Size = (UINTN)(FwVolHeader->FvLength - FwVolHeader->HeaderLength);
+  Size = (UINTN) FwVolHeader->FvLength;
   if ((FvbAttributes & EFI_FVB2_MEMORY_MAPPED) != 0) {
     FvDevice->IsMemoryMapped = TRUE;
 
@@ -369,7 +363,7 @@ FvCheck (
     //
     // Don't cache memory mapped FV really.
     //
-    FvDevice->CachedFv = (UINT8 *) (UINTN) (PhysicalAddress + FwVolHeader->HeaderLength);
+    FvDevice->CachedFv = (UINT8 *) (UINTN) PhysicalAddress;
   } else {
     FvDevice->IsMemoryMapped = FALSE;
     FvDevice->CachedFv = AllocatePool (Size);
@@ -380,52 +374,27 @@ FvCheck (
   }
 
   //
-  // Remember a pointer to the end fo the CachedFv
+  // Remember a pointer to the end of the CachedFv
   //
   FvDevice->EndOfCachedFv = FvDevice->CachedFv + Size;
 
   if (!FvDevice->IsMemoryMapped) {
     //
-    // Copy FV minus header into memory using the block map we have all ready
-    // read into memory.
+    // Copy FV into memory using the block map.
     //
     BlockMap = FwVolHeader->BlockMap;
     CacheLocation = FvDevice->CachedFv;
     LbaIndex = 0;
-    LbaOffset = 0;
-    HeaderSize = FwVolHeader->HeaderLength;
     while ((BlockMap->NumBlocks != 0) || (BlockMap->Length != 0)) {
-      Index = 0;
-      Size  = BlockMap->Length;
-      if (HeaderSize > 0) {
-        //
-        // Skip header size
-        //
-        for (; Index < BlockMap->NumBlocks && HeaderSize >= BlockMap->Length; Index ++) {
-          HeaderSize -= BlockMap->Length;
-          LbaIndex ++;
-        }
-
-        //
-        // Check whether FvHeader is crossing the multi block range.
-        //
-        if (Index >= BlockMap->NumBlocks) {
-          BlockMap++;
-          continue;
-        } else if (HeaderSize > 0) {
-          LbaOffset = HeaderSize;
-          Size = BlockMap->Length - HeaderSize;
-          HeaderSize = 0;
-        }
-      }
-
       //
       // read the FV data
       //
-      for (; Index < BlockMap->NumBlocks; Index ++) {
-        Status = Fvb->Read (Fvb,
+      Size = BlockMap->Length;
+      for (Index = 0; Index < BlockMap->NumBlocks; Index++) {
+        Status = Fvb->Read (
+                        Fvb,
                         LbaIndex,
-                        LbaOffset,
+                        0,
                         &Size,
                         CacheLocation
                         );
@@ -438,13 +407,7 @@ FvCheck (
         }
 
         LbaIndex++;
-        CacheLocation += Size;
-
-        //
-        // After we skip Fv Header always read from start of block
-        //
-        LbaOffset = 0;
-        Size  = BlockMap->Length;
+        CacheLocation += BlockMap->Length;
       }
 
       BlockMap++;
@@ -475,12 +438,12 @@ FvCheck (
     //
     // Searching for files starts on an 8 byte aligned boundary after the end of the Extended Header if it exists.
     //
-    FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (FvDevice->CachedFv + (FwVolHeader->ExtHeaderOffset - FwVolHeader->HeaderLength));
+    FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (FvDevice->CachedFv + FwVolHeader->ExtHeaderOffset);
     FfsHeader = (EFI_FFS_FILE_HEADER *) ((UINT8 *) FwVolExtHeader + FwVolExtHeader->ExtHeaderSize);
-    FfsHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsHeader, 8);
   } else {
-    FfsHeader = (EFI_FFS_FILE_HEADER *) (FvDevice->CachedFv);
+    FfsHeader = (EFI_FFS_FILE_HEADER *) (FvDevice->CachedFv + FwVolHeader->HeaderLength);
   }
+  FfsHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsHeader, 8);
   TopFvAddress = FvDevice->EndOfCachedFv;
   while (((UINTN) FfsHeader >= (UINTN) FvDevice->CachedFv) && ((UINTN) FfsHeader <= (UINTN) ((UINTN) TopFvAddress - sizeof (EFI_FFS_FILE_HEADER)))) {
 
-- 
2.17.1.windows.2



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

* [PATCH 3/3] IntelFrameworkModulePkg/FwVolDxe: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
  2019-02-26 16:04 [PATCH 0/3] Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630] Jian J Wang
  2019-02-26 16:04 ` [PATCH 1/3] MdeModulePkg/PeiCore: " Jian J Wang
  2019-02-26 16:04 ` [PATCH 2/3] MdeModulePkg/DxeCore: " Jian J Wang
@ 2019-02-26 16:04 ` Jian J Wang
  2019-02-27  6:55   ` Wang, Jian J
  2 siblings, 1 reply; 7+ messages in thread
From: Jian J Wang @ 2019-02-26 16:04 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Liming Gao

From: Star Zeng <star.zeng@intel.com>

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

To follow PI spec, ensure FfsFileHeader 8 bytes aligned.

Current code only handles (FwVolHeader->ExtHeaderOffset != 0) path,
update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Universal/FirmwareVolume/FwVolDxe/FwVol.c                 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c b/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
index 9a031bab14..9a892240b4 100644
--- a/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
+++ b/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
@@ -4,7 +4,7 @@
   Layers on top of Firmware Block protocol to produce a file abstraction
   of FV based files.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions
@@ -510,10 +510,10 @@ FvCheck (
     //
     FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (UINTN) (FvDevice->CachedFv + FvDevice->FwVolHeader->ExtHeaderOffset);
     Ptr = (UINT8 *) FwVolExtHeader + FwVolExtHeader->ExtHeaderSize;
-    Ptr = (UINT8 *) ALIGN_POINTER (Ptr, 8);
   } else {
     Ptr = (UINT8 *) (UINTN) (FvDevice->CachedFv + FvDevice->FwVolHeader->HeaderLength);
   }
+  Ptr = (UINT8 *) ALIGN_POINTER (Ptr, 8);
   TopFvAddress = (UINT8 *) (UINTN) (FvDevice->CachedFv + FvDevice->FwVolHeader->FvLength);
 
   //
-- 
2.17.1.windows.2



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

* Re: [PATCH 3/3] IntelFrameworkModulePkg/FwVolDxe: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
  2019-02-26 16:04 ` [PATCH 3/3] IntelFrameworkModulePkg/FwVolDxe: " Jian J Wang
@ 2019-02-27  6:55   ` Wang, Jian J
  0 siblings, 0 replies; 7+ messages in thread
From: Wang, Jian J @ 2019-02-27  6:55 UTC (permalink / raw)
  To: Wang, Jian J, edk2-devel@lists.01.org
  Cc: Yao, Jiewen, Gao, Liming, Zeng, Star

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Wednesday, February 27, 2019 12:04 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 3/3] IntelFrameworkModulePkg/FwVolDxe: Ensure
> FfsFileHeader 8 bytes aligned [CVE-2018-3630]
> 
> From: Star Zeng <star.zeng@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=864
> 
> To follow PI spec, ensure FfsFileHeader 8 bytes aligned.
> 
> Current code only handles (FwVolHeader->ExtHeaderOffset != 0) path,
> update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  .../Universal/FirmwareVolume/FwVolDxe/FwVol.c                 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
> b/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
> index 9a031bab14..9a892240b4 100644
> --- a/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
> +++
> b/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
> @@ -4,7 +4,7 @@
>    Layers on top of Firmware Block protocol to produce a file abstraction
>    of FV based files.
> 
> -  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions
> @@ -510,10 +510,10 @@ FvCheck (
>      //
>      FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (UINTN)
> (FvDevice->CachedFv + FvDevice->FwVolHeader->ExtHeaderOffset);
>      Ptr = (UINT8 *) FwVolExtHeader + FwVolExtHeader->ExtHeaderSize;
> -    Ptr = (UINT8 *) ALIGN_POINTER (Ptr, 8);
>    } else {
>      Ptr = (UINT8 *) (UINTN) (FvDevice->CachedFv + FvDevice->FwVolHeader-
> >HeaderLength);
>    }
> +  Ptr = (UINT8 *) ALIGN_POINTER (Ptr, 8);
>    TopFvAddress = (UINT8 *) (UINTN) (FvDevice->CachedFv + FvDevice-
> >FwVolHeader->FvLength);
> 
>    //
> --
> 2.17.1.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [PATCH 2/3] MdeModulePkg/DxeCore: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
  2019-02-26 16:04 ` [PATCH 2/3] MdeModulePkg/DxeCore: " Jian J Wang
@ 2019-02-27  6:55   ` Wang, Jian J
  0 siblings, 0 replies; 7+ messages in thread
From: Wang, Jian J @ 2019-02-27  6:55 UTC (permalink / raw)
  To: Wang, Jian J, edk2-devel@lists.01.org
  Cc: Wu, Hao A, Yao, Jiewen, Gao, Liming, Zeng, Star


Reviewed-by: Jian J Wang <jian.j.wang@intel.com>


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Wednesday, February 27, 2019 12:04 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 2/3] MdeModulePkg/DxeCore: Ensure FfsFileHeader 8
> bytes aligned [CVE-2018-3630]
> 
> From: Star Zeng <star.zeng@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=864
> 
> To follow PI spec, ensure FfsFileHeader 8 bytes aligned.
> 
> For the integrity of FV(especially non-MemoryMapped FV) layout,
> let CachedFv point to FV beginning, but not (FV + FV header).
> 
> And current code only handles (FwVolHeader->ExtHeaderOffset != 0) path,
> update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/FwVol/FwVol.c | 65 +++++++----------------------
>  1 file changed, 14 insertions(+), 51 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> index 93ddcc3591..28fce46a95 100644
> --- a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> +++ b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> @@ -3,7 +3,7 @@
>    Layers on top of Firmware Block protocol to produce a file abstraction
>    of FV based files.
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -329,8 +329,6 @@ FvCheck (
>    FFS_FILE_LIST_ENTRY                   *FfsFileEntry;
>    EFI_FFS_FILE_HEADER                   *FfsHeader;
>    UINT8                                 *CacheLocation;
> -  UINTN                                 LbaOffset;
> -  UINTN                                 HeaderSize;
>    UINTN                                 Index;
>    EFI_LBA                               LbaIndex;
>    UINTN                                 Size;
> @@ -353,11 +351,7 @@ FvCheck (
>      return Status;
>    }
> 
> -  //
> -  // Size is the size of the FV minus the head. We have already allocated
> -  // the header to check to make sure the volume is valid
> -  //
> -  Size = (UINTN)(FwVolHeader->FvLength - FwVolHeader->HeaderLength);
> +  Size = (UINTN) FwVolHeader->FvLength;
>    if ((FvbAttributes & EFI_FVB2_MEMORY_MAPPED) != 0) {
>      FvDevice->IsMemoryMapped = TRUE;
> 
> @@ -369,7 +363,7 @@ FvCheck (
>      //
>      // Don't cache memory mapped FV really.
>      //
> -    FvDevice->CachedFv = (UINT8 *) (UINTN) (PhysicalAddress + FwVolHeader-
> >HeaderLength);
> +    FvDevice->CachedFv = (UINT8 *) (UINTN) PhysicalAddress;
>    } else {
>      FvDevice->IsMemoryMapped = FALSE;
>      FvDevice->CachedFv = AllocatePool (Size);
> @@ -380,52 +374,27 @@ FvCheck (
>    }
> 
>    //
> -  // Remember a pointer to the end fo the CachedFv
> +  // Remember a pointer to the end of the CachedFv
>    //
>    FvDevice->EndOfCachedFv = FvDevice->CachedFv + Size;
> 
>    if (!FvDevice->IsMemoryMapped) {
>      //
> -    // Copy FV minus header into memory using the block map we have all ready
> -    // read into memory.
> +    // Copy FV into memory using the block map.
>      //
>      BlockMap = FwVolHeader->BlockMap;
>      CacheLocation = FvDevice->CachedFv;
>      LbaIndex = 0;
> -    LbaOffset = 0;
> -    HeaderSize = FwVolHeader->HeaderLength;
>      while ((BlockMap->NumBlocks != 0) || (BlockMap->Length != 0)) {
> -      Index = 0;
> -      Size  = BlockMap->Length;
> -      if (HeaderSize > 0) {
> -        //
> -        // Skip header size
> -        //
> -        for (; Index < BlockMap->NumBlocks && HeaderSize >= BlockMap->Length;
> Index ++) {
> -          HeaderSize -= BlockMap->Length;
> -          LbaIndex ++;
> -        }
> -
> -        //
> -        // Check whether FvHeader is crossing the multi block range.
> -        //
> -        if (Index >= BlockMap->NumBlocks) {
> -          BlockMap++;
> -          continue;
> -        } else if (HeaderSize > 0) {
> -          LbaOffset = HeaderSize;
> -          Size = BlockMap->Length - HeaderSize;
> -          HeaderSize = 0;
> -        }
> -      }
> -
>        //
>        // read the FV data
>        //
> -      for (; Index < BlockMap->NumBlocks; Index ++) {
> -        Status = Fvb->Read (Fvb,
> +      Size = BlockMap->Length;
> +      for (Index = 0; Index < BlockMap->NumBlocks; Index++) {
> +        Status = Fvb->Read (
> +                        Fvb,
>                          LbaIndex,
> -                        LbaOffset,
> +                        0,
>                          &Size,
>                          CacheLocation
>                          );
> @@ -438,13 +407,7 @@ FvCheck (
>          }
> 
>          LbaIndex++;
> -        CacheLocation += Size;
> -
> -        //
> -        // After we skip Fv Header always read from start of block
> -        //
> -        LbaOffset = 0;
> -        Size  = BlockMap->Length;
> +        CacheLocation += BlockMap->Length;
>        }
> 
>        BlockMap++;
> @@ -475,12 +438,12 @@ FvCheck (
>      //
>      // Searching for files starts on an 8 byte aligned boundary after the end of the
> Extended Header if it exists.
>      //
> -    FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (FvDevice-
> >CachedFv + (FwVolHeader->ExtHeaderOffset - FwVolHeader->HeaderLength));
> +    FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (FvDevice-
> >CachedFv + FwVolHeader->ExtHeaderOffset);
>      FfsHeader = (EFI_FFS_FILE_HEADER *) ((UINT8 *) FwVolExtHeader +
> FwVolExtHeader->ExtHeaderSize);
> -    FfsHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsHeader, 8);
>    } else {
> -    FfsHeader = (EFI_FFS_FILE_HEADER *) (FvDevice->CachedFv);
> +    FfsHeader = (EFI_FFS_FILE_HEADER *) (FvDevice->CachedFv + FwVolHeader-
> >HeaderLength);
>    }
> +  FfsHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsHeader, 8);
>    TopFvAddress = FvDevice->EndOfCachedFv;
>    while (((UINTN) FfsHeader >= (UINTN) FvDevice->CachedFv) && ((UINTN)
> FfsHeader <= (UINTN) ((UINTN) TopFvAddress - sizeof (EFI_FFS_FILE_HEADER))))
> {
> 
> --
> 2.17.1.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [PATCH 1/3] MdeModulePkg/PeiCore: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
  2019-02-26 16:04 ` [PATCH 1/3] MdeModulePkg/PeiCore: " Jian J Wang
@ 2019-02-27  6:56   ` Wang, Jian J
  0 siblings, 0 replies; 7+ messages in thread
From: Wang, Jian J @ 2019-02-27  6:56 UTC (permalink / raw)
  To: Wang, Jian J, edk2-devel@lists.01.org
  Cc: Wu, Hao A, Yao, Jiewen, Gao, Liming, Zeng, Star



Reviewed-by: Jian J Wang <jian.j.wang@intel.com>


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Wednesday, February 27, 2019 12:04 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 1/3] MdeModulePkg/PeiCore: Ensure FfsFileHeader 8
> bytes aligned [CVE-2018-3630]
> 
> From: Star Zeng <star.zeng@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=864
> 
> To follow PI spec, ensure FfsFileHeader 8 bytes aligned.
> 
> Current code only handles (FwVolHeader->ExtHeaderOffset != 0) path,
> update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/Pei/FwVol/FwVol.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> index 0a67b96bf1..56440eacf0 100644
> --- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> +++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> @@ -2,7 +2,7 @@
>    Pei Core Firmware File System service routines.
> 
>  Copyright (c) 2015 HP Development Company, L.P.
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -316,10 +316,10 @@ FindFileEx (
>        //
>        FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) ((UINT8 *)
> FwVolHeader + FwVolHeader->ExtHeaderOffset);
>        FfsFileHeader = (EFI_FFS_FILE_HEADER *) ((UINT8 *) FwVolExtHeader +
> FwVolExtHeader->ExtHeaderSize);
> -      FfsFileHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsFileHeader, 8);
>      } else {
>        FfsFileHeader = (EFI_FFS_FILE_HEADER *)((UINT8 *) FwVolHeader +
> FwVolHeader->HeaderLength);
>      }
> +    FfsFileHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsFileHeader, 8);
>    } else {
>      if (IS_FFS_FILE2 (*FileHeader)) {
>        if (!IsFfs3Fv) {
> --
> 2.17.1.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

end of thread, other threads:[~2019-02-27  6:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-26 16:04 [PATCH 0/3] Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630] Jian J Wang
2019-02-26 16:04 ` [PATCH 1/3] MdeModulePkg/PeiCore: " Jian J Wang
2019-02-27  6:56   ` Wang, Jian J
2019-02-26 16:04 ` [PATCH 2/3] MdeModulePkg/DxeCore: " Jian J Wang
2019-02-27  6:55   ` Wang, Jian J
2019-02-26 16:04 ` [PATCH 3/3] IntelFrameworkModulePkg/FwVolDxe: " Jian J Wang
2019-02-27  6:55   ` Wang, Jian J

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