[edk2-devel] [Patch v2 03/11] BaseTools GenFw: Fix the issue to update the wrong size as SectionSize

Liming Gao posted 11 patches 6 years, 3 months ago
There is a newer version of this series
[edk2-devel] [Patch v2 03/11] BaseTools GenFw: Fix the issue to update the wrong size as SectionSize
Posted by Liming Gao 6 years, 3 months ago
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 BaseTools/Source/C/GenFw/GenFw.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index c99782b78e..d8d3360c24 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -653,7 +653,11 @@ PeCoffConvertImageToXip (
     //
     // Make the size of raw data in section header alignment.
     //
-    SectionHeader->SizeOfRawData = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
+    SectionSize = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
+    if (SectionSize < SectionHeader->SizeOfRawData) {
+      SectionHeader->SizeOfRawData = SectionSize;
+    }
+    
     SectionHeader->PointerToRawData = SectionHeader->VirtualAddress;
   }
 
@@ -999,7 +1003,7 @@ Returns:
     CopyMem (
       FileBuffer + SectionHeader->PointerToRawData,
       (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress),
-      SectionHeader->SizeOfRawData
+      SectionHeader->SizeOfRawData < SectionHeader->Misc.VirtualSize ? SectionHeader->SizeOfRawData : SectionHeader->Misc.VirtualSize
       );
   }
 
-- 
2.13.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48955): https://edk2.groups.io/g/devel/message/48955
Mute This Topic: https://groups.io/mt/34540584/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v2 03/11] BaseTools GenFw: Fix the issue to update the wrong size as SectionSize
Posted by Philippe Mathieu-Daudé 6 years, 3 months ago
On 10/15/19 2:26 AM, Liming Gao wrote:
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> ---
>   BaseTools/Source/C/GenFw/GenFw.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
> index c99782b78e..d8d3360c24 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -653,7 +653,11 @@ PeCoffConvertImageToXip (
>       //
>       // Make the size of raw data in section header alignment.
>       //
> -    SectionHeader->SizeOfRawData = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
> +    SectionSize = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
> +    if (SectionSize < SectionHeader->SizeOfRawData) {
> +      SectionHeader->SizeOfRawData = SectionSize;
> +    }
> +
>       SectionHeader->PointerToRawData = SectionHeader->VirtualAddress;
>     }
>   
> @@ -999,7 +1003,7 @@ Returns:
>       CopyMem (
>         FileBuffer + SectionHeader->PointerToRawData,
>         (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress),
> -      SectionHeader->SizeOfRawData
> +      SectionHeader->SizeOfRawData < SectionHeader->Misc.VirtualSize ? SectionHeader->SizeOfRawData : SectionHeader->Misc.VirtualSize
>         );
>     }
>   
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48989): https://edk2.groups.io/g/devel/message/48989
Mute This Topic: https://groups.io/mt/34540584/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v2 03/11] BaseTools GenFw: Fix the issue to update the wrong size as SectionSize
Posted by Bob Feng 6 years, 3 months ago
Hi Liming,

Would you please add more description in commit message? This patch is in the patch set of CLANG9 enabling, but it's hard for me to see the direct relationship between CLANG9 and this fix and why the original code is wrong.

Thanks
Bob

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Liming Gao
Sent: Tuesday, October 15, 2019 8:27 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] [Patch v2 03/11] BaseTools GenFw: Fix the issue to update the wrong size as SectionSize

Signed-off-by: Liming Gao <liming.gao@intel.com>
---
 BaseTools/Source/C/GenFw/GenFw.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index c99782b78e..d8d3360c24 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -653,7 +653,11 @@ PeCoffConvertImageToXip (
     //
     // Make the size of raw data in section header alignment.
     //
-    SectionHeader->SizeOfRawData = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
+    SectionSize = (SectionHeader->Misc.VirtualSize + PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr->Pe32.OptionalHeader.FileAlignment - 1));
+    if (SectionSize < SectionHeader->SizeOfRawData) {
+      SectionHeader->SizeOfRawData = SectionSize;
+    }
+
     SectionHeader->PointerToRawData = SectionHeader->VirtualAddress;
   }
 
@@ -999,7 +1003,7 @@ Returns:
     CopyMem (
       FileBuffer + SectionHeader->PointerToRawData,
       (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader->VirtualAddress),
-      SectionHeader->SizeOfRawData
+      SectionHeader->SizeOfRawData < SectionHeader->Misc.VirtualSize ? SectionHeader->SizeOfRawData : SectionHeader->Misc.VirtualSize
       );
   }
 
-- 
2.13.0.windows.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49083): https://edk2.groups.io/g/devel/message/49083
Mute This Topic: https://groups.io/mt/34540584/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v2 03/11] BaseTools GenFw: Fix the issue to update the wrong size as SectionSize
Posted by Liming Gao 6 years, 3 months ago
OK. I will update the commit message to describe this issue. 

This change is to make sure section size is in section raw data scope. 

>-----Original Message-----
>From: Feng, Bob C
>Sent: Wednesday, October 16, 2019 5:27 PM
>To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
>Subject: RE: [edk2-devel] [Patch v2 03/11] BaseTools GenFw: Fix the issue to
>update the wrong size as SectionSize
>
>Hi Liming,
>
>Would you please add more description in commit message? This patch is in
>the patch set of CLANG9 enabling, but it's hard for me to see the direct
>relationship between CLANG9 and this fix and why the original code is wrong.
>
>Thanks
>Bob
>
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Tuesday, October 15, 2019 8:27 AM
>To: devel@edk2.groups.io
>Subject: [edk2-devel] [Patch v2 03/11] BaseTools GenFw: Fix the issue to
>update the wrong size as SectionSize
>
>Signed-off-by: Liming Gao <liming.gao@intel.com>
>---
> BaseTools/Source/C/GenFw/GenFw.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/BaseTools/Source/C/GenFw/GenFw.c
>b/BaseTools/Source/C/GenFw/GenFw.c
>index c99782b78e..d8d3360c24 100644
>--- a/BaseTools/Source/C/GenFw/GenFw.c
>+++ b/BaseTools/Source/C/GenFw/GenFw.c
>@@ -653,7 +653,11 @@ PeCoffConvertImageToXip (
>     //
>     // Make the size of raw data in section header alignment.
>     //
>-    SectionHeader->SizeOfRawData = (SectionHeader->Misc.VirtualSize +
>PeHdr->Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr-
>>Pe32.OptionalHeader.FileAlignment - 1));
>+    SectionSize = (SectionHeader->Misc.VirtualSize + PeHdr-
>>Pe32.OptionalHeader.FileAlignment - 1) & (~(PeHdr-
>>Pe32.OptionalHeader.FileAlignment - 1));
>+    if (SectionSize < SectionHeader->SizeOfRawData) {
>+      SectionHeader->SizeOfRawData = SectionSize;
>+    }
>+
>     SectionHeader->PointerToRawData = SectionHeader->VirtualAddress;
>   }
>
>@@ -999,7 +1003,7 @@ Returns:
>     CopyMem (
>       FileBuffer + SectionHeader->PointerToRawData,
>       (VOID*) (UINTN) (ImageContext.ImageAddress + SectionHeader-
>>VirtualAddress),
>-      SectionHeader->SizeOfRawData
>+      SectionHeader->SizeOfRawData < SectionHeader->Misc.VirtualSize ?
>SectionHeader->SizeOfRawData : SectionHeader->Misc.VirtualSize
>       );
>   }
>
>--
>2.13.0.windows.1
>
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49137): https://edk2.groups.io/g/devel/message/49137
Mute This Topic: https://groups.io/mt/34540584/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-