[edk2-devel] [PATCH] IntelFsp2Pkg/SplitFspBin.py: Command crashed when FV almost full.

Chiu, Chasel posted 1 patch 4 years, 6 months ago
Failed in applying to current master (apply log)
IntelFsp2Pkg/Tools/SplitFspBin.py | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[edk2-devel] [PATCH] IntelFsp2Pkg/SplitFspBin.py: Command crashed when FV almost full.
Posted by Chiu, Chasel 4 years, 6 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2310

When target FV 99% used and only few bytes space left,
SplitFspBin.py may crash with below error:

  File "SplitFspBin.py", line 457, in ParseFv
    ffshdr = EFI_FFS_FILE_HEADER.from_buffer (self.FvData, offset)
ValueError: Buffer size too small
  (40960 instead of at least 40968 bytes)

It was because the offset used by FFS_HEADER parser out of bounds.
It should stop parsing when offset equal or larger than
(buffer size + FFS_HEADER size).

This patch also fixed another crash issue when running script with
Python 3.x and no input parameter given:

  File "SplitFspBin.py", line 868, in main
    if args.which in ['rebase', 'split', 'genhdr', 'info']:
AttributeError: 'Namespace' object has no attribute 'which'

Test:
1. Ran script with both py2 and py3 with no input and no crash observed.
2. Compare the script result before and after the patch are identical.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 IntelFsp2Pkg/Tools/SplitFspBin.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py b/IntelFsp2Pkg/Tools/SplitFspBin.py
index b326241fa9..39a7ea7460 100644
--- a/IntelFsp2Pkg/Tools/SplitFspBin.py
+++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
@@ -428,7 +428,7 @@ class FirmwareFile:
         ffssize = len(self.FfsData)
         offset  = sizeof(self.FfsHdr)
         if self.FfsHdr.Name != '\xff' * 16:
-            while offset < ffssize:
+            while offset < (ffssize - sizeof (EFI_COMMON_SECTION_HEADER)):
                 sechdr = EFI_COMMON_SECTION_HEADER.from_buffer (self.FfsData, offset)
                 sec = Section (offset, self.FfsData[offset:offset + int(sechdr.Size)])
                 self.SecList.append(sec)
@@ -453,7 +453,7 @@ class FirmwareVolume:
         else:
             offset = self.FvHdr.HeaderLength
         offset = AlignPtr(offset)
-        while offset < fvsize:
+        while offset < (fvsize - sizeof (EFI_FFS_FILE_HEADER)):
             ffshdr = EFI_FFS_FILE_HEADER.from_buffer (self.FvData, offset)
             if (ffshdr.Name == '\xff' * 16) and (int(ffshdr.Size) == 0xFFFFFF):
                 offset = fvsize
@@ -515,7 +515,7 @@ class FirmwareDevice:
         offset = 0
         fdsize = len(self.FdData)
         self.FvList  = []
-        while offset < fdsize:
+        while offset < (fdsize - sizeof (EFI_FIRMWARE_VOLUME_HEADER)):
             fvh = EFI_FIRMWARE_VOLUME_HEADER.from_buffer (self.FdData, offset)
             if b'_FVH' != fvh.Signature:
                 raise Exception("ERROR: Invalid FV header !")
@@ -838,7 +838,7 @@ def RebaseFspBin (FspBinary, FspComponent, FspBase, OutputDir, OutputFile):
 
 def main ():
     parser     = argparse.ArgumentParser()
-    subparsers = parser.add_subparsers(title='commands')
+    subparsers = parser.add_subparsers(title='commands', dest="which")
 
     parser_rebase  = subparsers.add_parser('rebase',  help='rebase a FSP into a new base address')
     parser_rebase.set_defaults(which='rebase')
@@ -880,7 +880,7 @@ def main ():
     elif args.which == 'info':
         ShowFspInfo (args.FspBinary)
     else:
-        pass
+        parser.print_help()
 
     return 0
 
-- 
2.13.3.windows.1


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

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

Re: [edk2-devel] [PATCH] IntelFsp2Pkg/SplitFspBin.py: Command crashed when FV almost full.
Posted by Nate DeSimone 4 years, 6 months ago
Good Catch!

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, Chasel
Sent: Friday, October 25, 2019 2:56 AM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [edk2-devel] [PATCH] IntelFsp2Pkg/SplitFspBin.py: Command crashed when FV almost full.

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

When target FV 99% used and only few bytes space left, SplitFspBin.py may crash with below error:

  File "SplitFspBin.py", line 457, in ParseFv
    ffshdr = EFI_FFS_FILE_HEADER.from_buffer (self.FvData, offset)
ValueError: Buffer size too small
  (40960 instead of at least 40968 bytes)

It was because the offset used by FFS_HEADER parser out of bounds.
It should stop parsing when offset equal or larger than (buffer size + FFS_HEADER size).

This patch also fixed another crash issue when running script with Python 3.x and no input parameter given:

  File "SplitFspBin.py", line 868, in main
    if args.which in ['rebase', 'split', 'genhdr', 'info']:
AttributeError: 'Namespace' object has no attribute 'which'

Test:
1. Ran script with both py2 and py3 with no input and no crash observed.
2. Compare the script result before and after the patch are identical.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 IntelFsp2Pkg/Tools/SplitFspBin.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py b/IntelFsp2Pkg/Tools/SplitFspBin.py
index b326241fa9..39a7ea7460 100644
--- a/IntelFsp2Pkg/Tools/SplitFspBin.py
+++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
@@ -428,7 +428,7 @@ class FirmwareFile:
         ffssize = len(self.FfsData)
         offset  = sizeof(self.FfsHdr)
         if self.FfsHdr.Name != '\xff' * 16:
-            while offset < ffssize:
+            while offset < (ffssize - sizeof (EFI_COMMON_SECTION_HEADER)):
                 sechdr = EFI_COMMON_SECTION_HEADER.from_buffer (self.FfsData, offset)
                 sec = Section (offset, self.FfsData[offset:offset + int(sechdr.Size)])
                 self.SecList.append(sec) @@ -453,7 +453,7 @@ class FirmwareVolume:
         else:
             offset = self.FvHdr.HeaderLength
         offset = AlignPtr(offset)
-        while offset < fvsize:
+        while offset < (fvsize - sizeof (EFI_FFS_FILE_HEADER)):
             ffshdr = EFI_FFS_FILE_HEADER.from_buffer (self.FvData, offset)
             if (ffshdr.Name == '\xff' * 16) and (int(ffshdr.Size) == 0xFFFFFF):
                 offset = fvsize
@@ -515,7 +515,7 @@ class FirmwareDevice:
         offset = 0
         fdsize = len(self.FdData)
         self.FvList  = []
-        while offset < fdsize:
+        while offset < (fdsize - sizeof (EFI_FIRMWARE_VOLUME_HEADER)):
             fvh = EFI_FIRMWARE_VOLUME_HEADER.from_buffer (self.FdData, offset)
             if b'_FVH' != fvh.Signature:
                 raise Exception("ERROR: Invalid FV header !") @@ -838,7 +838,7 @@ def RebaseFspBin (FspBinary, FspComponent, FspBase, OutputDir, OutputFile):
 
 def main ():
     parser     = argparse.ArgumentParser()
-    subparsers = parser.add_subparsers(title='commands')
+    subparsers = parser.add_subparsers(title='commands', dest="which")
 
     parser_rebase  = subparsers.add_parser('rebase',  help='rebase a FSP into a new base address')
     parser_rebase.set_defaults(which='rebase')
@@ -880,7 +880,7 @@ def main ():
     elif args.which == 'info':
         ShowFspInfo (args.FspBinary)
     else:
-        pass
+        parser.print_help()
 
     return 0
 
--
2.13.3.windows.1





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

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