public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg/MemoryProtection: split protect and unprotect paths
@ 2017-03-21 13:49 Ard Biesheuvel
  2017-03-21 14:14 ` Yao, Jiewen
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2017-03-21 13:49 UTC (permalink / raw)
  To: edk2-devel, jiewen.yao; +Cc: star.zeng, feng.tian, liming.gao, Ard Biesheuvel

Currently, the PE/COFF image memory protection code uses the same code
paths for protecting and unprotecting an image. This is strange, since
unprotecting an image involves a single call into the CPU arch protocol
to clear the permission attributes of the entire range, and there is no
need to parse the PE/COFF headers again.

So let's store the ImageRecord entries in a linked list, so we can find
it again at unprotect time, and simply clear the permissions.

Note that this fixes a DEBUG hang on an ASSERT() that occurs when the
PE/COFF image fails to load, which causes UnprotectUefiImage() to be
invoked before the image is fully loaded.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

This is an alternative fix for the issue addressed by patch 

   'MdeModulePkg/DxeCore: ignore PdbPointer if ImageAddress == 0'

(https://lists.01.org/pipermail/edk2-devel/2017-March/008766.html)

 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 85 ++++++++------------
 1 file changed, 35 insertions(+), 50 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 451cc35b9290..93f96f0c9502 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -74,6 +74,8 @@ UINT32   mImageProtectionPolicy;
 
 extern LIST_ENTRY         mGcdMemorySpaceMap;
 
+STATIC LIST_ENTRY         mProtectedImageRecordList;
+
 /**
   Sort code section in image record, based upon CodeSegmentBase from low to high.
 
@@ -238,13 +240,10 @@ SetUefiImageMemoryAttributes (
   Set UEFI image protection attributes.
 
   @param[in]  ImageRecord    A UEFI image record
-  @param[in]  Protect        TRUE:  Protect the UEFI image.
-                             FALSE: Unprotect the UEFI image.
 **/
 VOID
 SetUefiImageProtectionAttributes (
-  IN IMAGE_PROPERTIES_RECORD     *ImageRecord,
-  IN BOOLEAN                     Protect
+  IN IMAGE_PROPERTIES_RECORD     *ImageRecord
   )
 {
   IMAGE_PROPERTIES_RECORD_CODE_SECTION      *ImageRecordCodeSection;
@@ -253,7 +252,6 @@ SetUefiImageProtectionAttributes (
   LIST_ENTRY                                *ImageRecordCodeSectionList;
   UINT64                                    CurrentBase;
   UINT64                                    ImageEnd;
-  UINT64                                    Attribute;
 
   ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
 
@@ -276,29 +274,19 @@ SetUefiImageProtectionAttributes (
       //
       // DATA
       //
-      if (Protect) {
-        Attribute = EFI_MEMORY_XP;
-      } else {
-        Attribute = 0;
-      }
       SetUefiImageMemoryAttributes (
         CurrentBase,
         ImageRecordCodeSection->CodeSegmentBase - CurrentBase,
-        Attribute
+        EFI_MEMORY_XP
         );
     }
     //
     // CODE
     //
-    if (Protect) {
-      Attribute = EFI_MEMORY_RO;
-    } else {
-      Attribute = 0;
-    }
     SetUefiImageMemoryAttributes (
       ImageRecordCodeSection->CodeSegmentBase,
       ImageRecordCodeSection->CodeSegmentSize,
-      Attribute
+      EFI_MEMORY_RO
       );
     CurrentBase = ImageRecordCodeSection->CodeSegmentBase + ImageRecordCodeSection->CodeSegmentSize;
   }
@@ -310,15 +298,10 @@ SetUefiImageProtectionAttributes (
     //
     // DATA
     //
-    if (Protect) {
-      Attribute = EFI_MEMORY_XP;
-    } else {
-      Attribute = 0;
-    }
     SetUefiImageMemoryAttributes (
       CurrentBase,
       ImageEnd - CurrentBase,
-      Attribute
+      EFI_MEMORY_XP
       );
   }
   return ;
@@ -401,18 +384,15 @@ FreeImageRecord (
 }
 
 /**
-  Protect or unprotect UEFI image common function.
+  Protect UEFI PE/COFF image
 
   @param[in]  LoadedImage              The loaded image protocol
   @param[in]  LoadedImageDevicePath    The loaded image device path protocol
-  @param[in]  Protect                  TRUE:  Protect the UEFI image.
-                                       FALSE: Unprotect the UEFI image.
 **/
 VOID
-ProtectUefiImageCommon (
+ProtectUefiImage (
   IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
-  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath,
-  IN BOOLEAN                     Protect
+  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
   )
 {
   VOID                                 *ImageAddress;
@@ -617,35 +597,18 @@ ProtectUefiImageCommon (
   //
   // CPU ARCH present. Update memory attribute directly.
   //
-  SetUefiImageProtectionAttributes (ImageRecord, Protect);
+  SetUefiImageProtectionAttributes (ImageRecord);
 
   //
-  // Clean up
+  // Record the image record in the list so we can undo the protections later
   //
-  FreeImageRecord (ImageRecord);
+  InsertTailList (&mProtectedImageRecordList, &ImageRecord->Link);
 
 Finish:
   return ;
 }
 
 /**
-  Protect UEFI image.
-
-  @param[in]  LoadedImage              The loaded image protocol
-  @param[in]  LoadedImageDevicePath    The loaded image device path protocol
-**/
-VOID
-ProtectUefiImage (
-  IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
-  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
-  )
-{
-  if (PcdGet32(PcdImageProtectionPolicy) != 0) {
-    ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, TRUE);
-  }
-}
-
-/**
   Unprotect UEFI image.
 
   @param[in]  LoadedImage              The loaded image protocol
@@ -657,8 +620,28 @@ UnprotectUefiImage (
   IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
   )
 {
+  IMAGE_PROPERTIES_RECORD    *ImageRecord;
+  LIST_ENTRY                 *ImageRecordLink;
+
   if (PcdGet32(PcdImageProtectionPolicy) != 0) {
-    ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, FALSE);
+    for (ImageRecordLink = mProtectedImageRecordList.ForwardLink;
+         ImageRecordLink != &mProtectedImageRecordList;
+         ImageRecordLink = ImageRecordLink->ForwardLink) {
+      ImageRecord = CR (
+                      ImageRecordLink,
+                      IMAGE_PROPERTIES_RECORD,
+                      Link,
+                      IMAGE_PROPERTIES_RECORD_SIGNATURE
+                      );
+
+      if (ImageRecord->ImageBase == (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase) {
+        SetUefiImageMemoryAttributes (ImageRecord->ImageBase,
+                                      ImageRecord->ImageSize,
+                                      0);
+        FreeImageRecord (ImageRecord);
+        return;
+      }
+    }
   }
 }
 
@@ -1027,6 +1010,8 @@ CoreInitializeMemoryProtection (
 
   mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy);
 
+  InitializeListHead (&mProtectedImageRecordList);
+
   //
   // Sanity check the PcdDxeNxMemoryProtectionPolicy setting:
   // - code regions should have no EFI_MEMORY_XP attribute
-- 
2.7.4



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

* Re: [PATCH] MdeModulePkg/MemoryProtection: split protect and unprotect paths
  2017-03-21 13:49 [PATCH] MdeModulePkg/MemoryProtection: split protect and unprotect paths Ard Biesheuvel
@ 2017-03-21 14:14 ` Yao, Jiewen
  2017-03-21 21:30   ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Yao, Jiewen @ 2017-03-21 14:14 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org
  Cc: Zeng, Star, Tian, Feng, Gao, Liming

Good idea.

Reviewed-by: Jiewen.yao@Intel.com



> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 21, 2017 9:49 PM
> To: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>; Tian, Feng <feng.tian@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [PATCH] MdeModulePkg/MemoryProtection: split protect and
> unprotect paths
> 
> Currently, the PE/COFF image memory protection code uses the same code
> paths for protecting and unprotecting an image. This is strange, since
> unprotecting an image involves a single call into the CPU arch protocol
> to clear the permission attributes of the entire range, and there is no
> need to parse the PE/COFF headers again.
> 
> So let's store the ImageRecord entries in a linked list, so we can find
> it again at unprotect time, and simply clear the permissions.
> 
> Note that this fixes a DEBUG hang on an ASSERT() that occurs when the
> PE/COFF image fails to load, which causes UnprotectUefiImage() to be
> invoked before the image is fully loaded.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> 
> This is an alternative fix for the issue addressed by patch
> 
>    'MdeModulePkg/DxeCore: ignore PdbPointer if ImageAddress == 0'
> 
> (https://lists.01.org/pipermail/edk2-devel/2017-March/008766.html)
> 
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 85 ++++++++------------
>  1 file changed, 35 insertions(+), 50 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index 451cc35b9290..93f96f0c9502 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -74,6 +74,8 @@ UINT32   mImageProtectionPolicy;
> 
>  extern LIST_ENTRY         mGcdMemorySpaceMap;
> 
> +STATIC LIST_ENTRY         mProtectedImageRecordList;
> +
>  /**
>    Sort code section in image record, based upon CodeSegmentBase from low to
> high.
> 
> @@ -238,13 +240,10 @@ SetUefiImageMemoryAttributes (
>    Set UEFI image protection attributes.
> 
>    @param[in]  ImageRecord    A UEFI image record
> -  @param[in]  Protect        TRUE:  Protect the UEFI image.
> -                             FALSE: Unprotect the UEFI image.
>  **/
>  VOID
>  SetUefiImageProtectionAttributes (
> -  IN IMAGE_PROPERTIES_RECORD     *ImageRecord,
> -  IN BOOLEAN                     Protect
> +  IN IMAGE_PROPERTIES_RECORD     *ImageRecord
>    )
>  {
>    IMAGE_PROPERTIES_RECORD_CODE_SECTION
> *ImageRecordCodeSection;
> @@ -253,7 +252,6 @@ SetUefiImageProtectionAttributes (
>    LIST_ENTRY
> *ImageRecordCodeSectionList;
>    UINT64                                    CurrentBase;
>    UINT64                                    ImageEnd;
> -  UINT64                                    Attribute;
> 
>    ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
> 
> @@ -276,29 +274,19 @@ SetUefiImageProtectionAttributes (
>        //
>        // DATA
>        //
> -      if (Protect) {
> -        Attribute = EFI_MEMORY_XP;
> -      } else {
> -        Attribute = 0;
> -      }
>        SetUefiImageMemoryAttributes (
>          CurrentBase,
>          ImageRecordCodeSection->CodeSegmentBase - CurrentBase,
> -        Attribute
> +        EFI_MEMORY_XP
>          );
>      }
>      //
>      // CODE
>      //
> -    if (Protect) {
> -      Attribute = EFI_MEMORY_RO;
> -    } else {
> -      Attribute = 0;
> -    }
>      SetUefiImageMemoryAttributes (
>        ImageRecordCodeSection->CodeSegmentBase,
>        ImageRecordCodeSection->CodeSegmentSize,
> -      Attribute
> +      EFI_MEMORY_RO
>        );
>      CurrentBase = ImageRecordCodeSection->CodeSegmentBase +
> ImageRecordCodeSection->CodeSegmentSize;
>    }
> @@ -310,15 +298,10 @@ SetUefiImageProtectionAttributes (
>      //
>      // DATA
>      //
> -    if (Protect) {
> -      Attribute = EFI_MEMORY_XP;
> -    } else {
> -      Attribute = 0;
> -    }
>      SetUefiImageMemoryAttributes (
>        CurrentBase,
>        ImageEnd - CurrentBase,
> -      Attribute
> +      EFI_MEMORY_XP
>        );
>    }
>    return ;
> @@ -401,18 +384,15 @@ FreeImageRecord (
>  }
> 
>  /**
> -  Protect or unprotect UEFI image common function.
> +  Protect UEFI PE/COFF image
> 
>    @param[in]  LoadedImage              The loaded image protocol
>    @param[in]  LoadedImageDevicePath    The loaded image device path
> protocol
> -  @param[in]  Protect                  TRUE:  Protect the UEFI image.
> -                                       FALSE: Unprotect the UEFI image.
>  **/
>  VOID
> -ProtectUefiImageCommon (
> +ProtectUefiImage (
>    IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
> -  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath,
> -  IN BOOLEAN                     Protect
> +  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
>    )
>  {
>    VOID                                 *ImageAddress;
> @@ -617,35 +597,18 @@ ProtectUefiImageCommon (
>    //
>    // CPU ARCH present. Update memory attribute directly.
>    //
> -  SetUefiImageProtectionAttributes (ImageRecord, Protect);
> +  SetUefiImageProtectionAttributes (ImageRecord);
> 
>    //
> -  // Clean up
> +  // Record the image record in the list so we can undo the protections later
>    //
> -  FreeImageRecord (ImageRecord);
> +  InsertTailList (&mProtectedImageRecordList, &ImageRecord->Link);
> 
>  Finish:
>    return ;
>  }
> 
>  /**
> -  Protect UEFI image.
> -
> -  @param[in]  LoadedImage              The loaded image protocol
> -  @param[in]  LoadedImageDevicePath    The loaded image device path
> protocol
> -**/
> -VOID
> -ProtectUefiImage (
> -  IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
> -  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
> -  )
> -{
> -  if (PcdGet32(PcdImageProtectionPolicy) != 0) {
> -    ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath,
> TRUE);
> -  }
> -}
> -
> -/**
>    Unprotect UEFI image.
> 
>    @param[in]  LoadedImage              The loaded image protocol
> @@ -657,8 +620,28 @@ UnprotectUefiImage (
>    IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
>    )
>  {
> +  IMAGE_PROPERTIES_RECORD    *ImageRecord;
> +  LIST_ENTRY                 *ImageRecordLink;
> +
>    if (PcdGet32(PcdImageProtectionPolicy) != 0) {
> -    ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath,
> FALSE);
> +    for (ImageRecordLink = mProtectedImageRecordList.ForwardLink;
> +         ImageRecordLink != &mProtectedImageRecordList;
> +         ImageRecordLink = ImageRecordLink->ForwardLink) {
> +      ImageRecord = CR (
> +                      ImageRecordLink,
> +                      IMAGE_PROPERTIES_RECORD,
> +                      Link,
> +                      IMAGE_PROPERTIES_RECORD_SIGNATURE
> +                      );
> +
> +      if (ImageRecord->ImageBase ==
> (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase) {
> +        SetUefiImageMemoryAttributes (ImageRecord->ImageBase,
> +                                      ImageRecord->ImageSize,
> +                                      0);
> +        FreeImageRecord (ImageRecord);
> +        return;
> +      }
> +    }
>    }
>  }
> 
> @@ -1027,6 +1010,8 @@ CoreInitializeMemoryProtection (
> 
>    mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy);
> 
> +  InitializeListHead (&mProtectedImageRecordList);
> +
>    //
>    // Sanity check the PcdDxeNxMemoryProtectionPolicy setting:
>    // - code regions should have no EFI_MEMORY_XP attribute
> --
> 2.7.4



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

* Re: [PATCH] MdeModulePkg/MemoryProtection: split protect and unprotect paths
  2017-03-21 14:14 ` Yao, Jiewen
@ 2017-03-21 21:30   ` Ard Biesheuvel
  0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2017-03-21 21:30 UTC (permalink / raw)
  To: Yao, Jiewen; +Cc: edk2-devel@lists.01.org, Zeng, Star, Tian, Feng, Gao, Liming

On 21 March 2017 at 14:14, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Good idea.
>
> Reviewed-by: Jiewen.yao@Intel.com
>

Pushed, thanks.


>
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Tuesday, March 21, 2017 9:49 PM
>> To: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>
>> Cc: Zeng, Star <star.zeng@intel.com>; Tian, Feng <feng.tian@intel.com>; Gao,
>> Liming <liming.gao@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: [PATCH] MdeModulePkg/MemoryProtection: split protect and
>> unprotect paths
>>
>> Currently, the PE/COFF image memory protection code uses the same code
>> paths for protecting and unprotecting an image. This is strange, since
>> unprotecting an image involves a single call into the CPU arch protocol
>> to clear the permission attributes of the entire range, and there is no
>> need to parse the PE/COFF headers again.
>>
>> So let's store the ImageRecord entries in a linked list, so we can find
>> it again at unprotect time, and simply clear the permissions.
>>
>> Note that this fixes a DEBUG hang on an ASSERT() that occurs when the
>> PE/COFF image fails to load, which causes UnprotectUefiImage() to be
>> invoked before the image is fully loaded.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>
>> This is an alternative fix for the issue addressed by patch
>>
>>    'MdeModulePkg/DxeCore: ignore PdbPointer if ImageAddress == 0'
>>
>> (https://lists.01.org/pipermail/edk2-devel/2017-March/008766.html)
>>
>>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 85 ++++++++------------
>>  1 file changed, 35 insertions(+), 50 deletions(-)
>>
>> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> index 451cc35b9290..93f96f0c9502 100644
>> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> @@ -74,6 +74,8 @@ UINT32   mImageProtectionPolicy;
>>
>>  extern LIST_ENTRY         mGcdMemorySpaceMap;
>>
>> +STATIC LIST_ENTRY         mProtectedImageRecordList;
>> +
>>  /**
>>    Sort code section in image record, based upon CodeSegmentBase from low to
>> high.
>>
>> @@ -238,13 +240,10 @@ SetUefiImageMemoryAttributes (
>>    Set UEFI image protection attributes.
>>
>>    @param[in]  ImageRecord    A UEFI image record
>> -  @param[in]  Protect        TRUE:  Protect the UEFI image.
>> -                             FALSE: Unprotect the UEFI image.
>>  **/
>>  VOID
>>  SetUefiImageProtectionAttributes (
>> -  IN IMAGE_PROPERTIES_RECORD     *ImageRecord,
>> -  IN BOOLEAN                     Protect
>> +  IN IMAGE_PROPERTIES_RECORD     *ImageRecord
>>    )
>>  {
>>    IMAGE_PROPERTIES_RECORD_CODE_SECTION
>> *ImageRecordCodeSection;
>> @@ -253,7 +252,6 @@ SetUefiImageProtectionAttributes (
>>    LIST_ENTRY
>> *ImageRecordCodeSectionList;
>>    UINT64                                    CurrentBase;
>>    UINT64                                    ImageEnd;
>> -  UINT64                                    Attribute;
>>
>>    ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
>>
>> @@ -276,29 +274,19 @@ SetUefiImageProtectionAttributes (
>>        //
>>        // DATA
>>        //
>> -      if (Protect) {
>> -        Attribute = EFI_MEMORY_XP;
>> -      } else {
>> -        Attribute = 0;
>> -      }
>>        SetUefiImageMemoryAttributes (
>>          CurrentBase,
>>          ImageRecordCodeSection->CodeSegmentBase - CurrentBase,
>> -        Attribute
>> +        EFI_MEMORY_XP
>>          );
>>      }
>>      //
>>      // CODE
>>      //
>> -    if (Protect) {
>> -      Attribute = EFI_MEMORY_RO;
>> -    } else {
>> -      Attribute = 0;
>> -    }
>>      SetUefiImageMemoryAttributes (
>>        ImageRecordCodeSection->CodeSegmentBase,
>>        ImageRecordCodeSection->CodeSegmentSize,
>> -      Attribute
>> +      EFI_MEMORY_RO
>>        );
>>      CurrentBase = ImageRecordCodeSection->CodeSegmentBase +
>> ImageRecordCodeSection->CodeSegmentSize;
>>    }
>> @@ -310,15 +298,10 @@ SetUefiImageProtectionAttributes (
>>      //
>>      // DATA
>>      //
>> -    if (Protect) {
>> -      Attribute = EFI_MEMORY_XP;
>> -    } else {
>> -      Attribute = 0;
>> -    }
>>      SetUefiImageMemoryAttributes (
>>        CurrentBase,
>>        ImageEnd - CurrentBase,
>> -      Attribute
>> +      EFI_MEMORY_XP
>>        );
>>    }
>>    return ;
>> @@ -401,18 +384,15 @@ FreeImageRecord (
>>  }
>>
>>  /**
>> -  Protect or unprotect UEFI image common function.
>> +  Protect UEFI PE/COFF image
>>
>>    @param[in]  LoadedImage              The loaded image protocol
>>    @param[in]  LoadedImageDevicePath    The loaded image device path
>> protocol
>> -  @param[in]  Protect                  TRUE:  Protect the UEFI image.
>> -                                       FALSE: Unprotect the UEFI image.
>>  **/
>>  VOID
>> -ProtectUefiImageCommon (
>> +ProtectUefiImage (
>>    IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
>> -  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath,
>> -  IN BOOLEAN                     Protect
>> +  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
>>    )
>>  {
>>    VOID                                 *ImageAddress;
>> @@ -617,35 +597,18 @@ ProtectUefiImageCommon (
>>    //
>>    // CPU ARCH present. Update memory attribute directly.
>>    //
>> -  SetUefiImageProtectionAttributes (ImageRecord, Protect);
>> +  SetUefiImageProtectionAttributes (ImageRecord);
>>
>>    //
>> -  // Clean up
>> +  // Record the image record in the list so we can undo the protections later
>>    //
>> -  FreeImageRecord (ImageRecord);
>> +  InsertTailList (&mProtectedImageRecordList, &ImageRecord->Link);
>>
>>  Finish:
>>    return ;
>>  }
>>
>>  /**
>> -  Protect UEFI image.
>> -
>> -  @param[in]  LoadedImage              The loaded image protocol
>> -  @param[in]  LoadedImageDevicePath    The loaded image device path
>> protocol
>> -**/
>> -VOID
>> -ProtectUefiImage (
>> -  IN EFI_LOADED_IMAGE_PROTOCOL   *LoadedImage,
>> -  IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
>> -  )
>> -{
>> -  if (PcdGet32(PcdImageProtectionPolicy) != 0) {
>> -    ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath,
>> TRUE);
>> -  }
>> -}
>> -
>> -/**
>>    Unprotect UEFI image.
>>
>>    @param[in]  LoadedImage              The loaded image protocol
>> @@ -657,8 +620,28 @@ UnprotectUefiImage (
>>    IN EFI_DEVICE_PATH_PROTOCOL    *LoadedImageDevicePath
>>    )
>>  {
>> +  IMAGE_PROPERTIES_RECORD    *ImageRecord;
>> +  LIST_ENTRY                 *ImageRecordLink;
>> +
>>    if (PcdGet32(PcdImageProtectionPolicy) != 0) {
>> -    ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath,
>> FALSE);
>> +    for (ImageRecordLink = mProtectedImageRecordList.ForwardLink;
>> +         ImageRecordLink != &mProtectedImageRecordList;
>> +         ImageRecordLink = ImageRecordLink->ForwardLink) {
>> +      ImageRecord = CR (
>> +                      ImageRecordLink,
>> +                      IMAGE_PROPERTIES_RECORD,
>> +                      Link,
>> +                      IMAGE_PROPERTIES_RECORD_SIGNATURE
>> +                      );
>> +
>> +      if (ImageRecord->ImageBase ==
>> (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase) {
>> +        SetUefiImageMemoryAttributes (ImageRecord->ImageBase,
>> +                                      ImageRecord->ImageSize,
>> +                                      0);
>> +        FreeImageRecord (ImageRecord);
>> +        return;
>> +      }
>> +    }
>>    }
>>  }
>>
>> @@ -1027,6 +1010,8 @@ CoreInitializeMemoryProtection (
>>
>>    mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy);
>>
>> +  InitializeListHead (&mProtectedImageRecordList);
>> +
>>    //
>>    // Sanity check the PcdDxeNxMemoryProtectionPolicy setting:
>>    // - code regions should have no EFI_MEMORY_XP attribute
>> --
>> 2.7.4
>


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

end of thread, other threads:[~2017-03-21 21:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-21 13:49 [PATCH] MdeModulePkg/MemoryProtection: split protect and unprotect paths Ard Biesheuvel
2017-03-21 14:14 ` Yao, Jiewen
2017-03-21 21:30   ` Ard Biesheuvel

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