public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 00/52] Resolve issues for C source codes in BaseTools
@ 2016-10-12 12:19 Hao Wu
  2016-10-12 12:19 ` [PATCH 01/52] BaseTools/C/Common: Avoid possible NULL pointer dereference Hao Wu
                   ` (52 more replies)
  0 siblings, 53 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

The patch series fixes the following types of issues for C source codes in
BaseTools:

1. Avoid possible NULL pointer dereference
2. Initialize local variables before use
3. Remove unused local variables
4. Avoid accessing over array bounds
5. Resolve possible memory leak
6. Resolve file handles not being closed
7. Resolve possible buffer overflow in printf/scanf functions

The patch series is also available at:
https://github.com/hwu25/edk2/tree/BaseTools_V1

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>

Hao Wu (52):
  BaseTools/C/Common: Avoid possible NULL pointer dereference
  BaseTools/EfiRom: Avoid possible NULL pointer dereference
  BaseTools/GenFfs: Avoid possible NULL pointer dereference
  BaseTools/GenFv: Avoid possible NULL pointer dereference
  BaseTools/GenFw: Avoid possible NULL pointer dereference
  BaseTools/GenPage: Avoid possible NULL pointer dereference
  BaseTools/GenSec: Avoid possible NULL pointer dereference
  BaseTools/GenVtf: Avoid possible NULL pointer dereference
  BaseTools/TianoCompress: Avoid possible NULL pointer dereference
  BaseTools/VfrCompile: Avoid possible NULL pointer dereference
  BaseTools/VolInfo: Avoid possible NULL pointer dereference
  BaseTools/TianoCompress: Initialize local variables before being used
  BaseTools/VfrCompile: Initialize local variables before being used
  BaseTools/GenBootSector: Fix parameter format mismatch in printf
    functions
  BaseTools/VolInfo: Fix parameter format mismatch in printf functions
  BaseTools/C/Common: Fix parameter format mismatch in scanf functions
  BaseTools/GenFv: Fix parameter format mismatch in scanf functions
  BaseTools/GenFw: Fix parameter format mismatch in scanf functions
  BaseTools/GenVtf: Fix parameter format mismatch in scanf functions
  BaseTools/C/Common: Fix potential access over array bounds
  BaseTools/EfiRom: Fix potential access over array bounds
  BaseTools/GenFv: Fix potential access over array bounds
  BaseTools/TianoCompress: Fix potential access over array bounds
  BaseTools/VfrCompile: Fix potential access over array bounds
  BaseTools/VfrCompile: Avoid freeing memory with mismatched functions
  BaseTools/VfrCompile: Add assignment operator definition for some
    classes
  BaseTools/VfrCompile: Avoid freeing freed memory in classes
  BaseTools/VfrCompile: Remove unused local variables
  BaseTools/C/Common: Fix potential memory leak
  BaseTools/EfiRom: Fix potential memory leak
  BaseTools/GenFv: Fix potential memory leak
  BaseTools/GenPage: Fix potential memory leak
  BaseTools/GenSec: Fix potential memory leak
  BaseTools/GenVtf: Fix potential memory leak
  BaseTools/Split: Fix potential memory and resource leak
  BaseTools/TianoCompress: Fix potential memory leak
  BaseTools/VfrCompile: Fix potential memory leak
  BaseTools/VolInfo: Fix potential memory leak
  BaseTools/EfiRom: Fix file handles not being closed
  BaseTools/GenBootSector: Fix file handles not being closed
  BaseTools/GenCrc32: Fix file handles not being closed
  BaseTools/GenFv: Fix file handles not being closed
  BaseTools/GenVtf: Fix file handles not being closed
  BaseTools/LzmaCompress: Fix file handles not being closed
  BaseTools/TianoCompress: Fix file handles not being closed
  BaseTools/VolInfo: Fix file handles not being closed
  BaseTools/GenVtf: Fix potential buffer overflow in scanf functions
  BaseTools/VolInfo: Fix potential buffer overflow in scanf functions
  BaseTools/VfrCompile: Explicitly state format string for DebugMsg()
  BaseTools/VolInfo: Use hard-coded format string for calls to sprintf()
  BaseTools/VfrCompile/Pccts: Add virtual destructor for class
    DLGInputStream
  BaseTools/VfrCompile/Pccts: Make assignment operator not returning
    void

 BaseTools/Source/C/Common/BasePeCoff.c             |  12 ++
 BaseTools/Source/C/Common/CommonLib.c              |   8 +-
 BaseTools/Source/C/Common/Decompress.c             |  41 ++++--
 BaseTools/Source/C/Common/EfiUtilityMsgs.c         |  20 +--
 BaseTools/Source/C/Common/FirmwareVolumeBuffer.c   |   6 +-
 BaseTools/Source/C/Common/MemoryFile.c             |   3 +-
 BaseTools/Source/C/Common/MyAlloc.c                |  55 +++++++-
 .../Source/C/Common/ParseGuidedSectionTools.c      |  21 ++--
 BaseTools/Source/C/Common/ParseInf.c               |  24 ++--
 BaseTools/Source/C/Common/SimpleFileParsing.c      |  14 +--
 BaseTools/Source/C/Common/TianoCompress.c          |   9 +-
 BaseTools/Source/C/EfiRom/EfiRom.c                 | 120 ++++++++++++------
 BaseTools/Source/C/GenBootSector/GenBootSector.c   |  43 ++++---
 BaseTools/Source/C/GenCrc32/GenCrc32.c             |   3 +-
 BaseTools/Source/C/GenFfs/GenFfs.c                 |  36 +++---
 BaseTools/Source/C/GenFv/GenFv.c                   |   9 +-
 BaseTools/Source/C/GenFv/GenFvInternalLib.c        |  83 ++++++++++--
 BaseTools/Source/C/GenFw/Elf32Convert.c            |   8 ++
 BaseTools/Source/C/GenFw/Elf64Convert.c            |  10 +-
 BaseTools/Source/C/GenFw/ElfConvert.c              |   7 +-
 BaseTools/Source/C/GenFw/GenFw.c                   |  20 ++-
 BaseTools/Source/C/GenPage/GenPage.c               |  12 +-
 BaseTools/Source/C/GenSec/GenSec.c                 |  27 +++-
 BaseTools/Source/C/GenVtf/GenVtf.c                 | 117 ++++++++++++++++-
 BaseTools/Source/C/LzmaCompress/LzmaCompress.c     |   6 +-
 BaseTools/Source/C/Split/Split.c                   |  41 ++++--
 BaseTools/Source/C/TianoCompress/TianoCompress.c   |  68 ++++++----
 BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h    |   4 +-
 .../Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h      |   6 +-
 BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h     |   3 +
 BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h |   4 +
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp      | 140 ++++++++++++++++++---
 BaseTools/Source/C/VfrCompile/VfrCompiler.h        |   8 +-
 BaseTools/Source/C/VfrCompile/VfrError.cpp         |   4 +-
 BaseTools/Source/C/VfrCompile/VfrError.h           |  10 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp       |  29 +++--
 BaseTools/Source/C/VfrCompile/VfrFormPkg.h         |  13 ++
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp    |  54 +++++---
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h      |  60 ++++++++-
 BaseTools/Source/C/VolInfo/VolInfo.c               | 107 +++++++++++++---
 40 files changed, 1004 insertions(+), 261 deletions(-)

-- 
1.9.5.msysgit.0



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

* [PATCH 01/52] BaseTools/C/Common: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 02/52] BaseTools/EfiRom: " Hao Wu
                   ` (51 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/Common/BasePeCoff.c             | 12 +++++
 BaseTools/Source/C/Common/EfiUtilityMsgs.c         | 20 ++++----
 BaseTools/Source/C/Common/FirmwareVolumeBuffer.c   |  5 +-
 BaseTools/Source/C/Common/MyAlloc.c                | 55 ++++++++++++++++++++--
 .../Source/C/Common/ParseGuidedSectionTools.c      | 15 +++---
 BaseTools/Source/C/Common/TianoCompress.c          |  9 +++-
 6 files changed, 93 insertions(+), 23 deletions(-)

diff --git a/BaseTools/Source/C/Common/BasePeCoff.c b/BaseTools/Source/C/Common/BasePeCoff.c
index d0cc1af..9adbdfa 100644
--- a/BaseTools/Source/C/Common/BasePeCoff.c
+++ b/BaseTools/Source/C/Common/BasePeCoff.c
@@ -650,6 +650,10 @@ Returns:
                         ImageContext,
                         RelocDir->VirtualAddress + RelocDir->Size - 1
                         );
+        if (RelocBase == NULL || RelocBaseEnd == NULL || RelocBaseEnd < RelocBase) {
+          ImageContext->ImageError = IMAGE_ERROR_FAILED_RELOCATION;
+          return RETURN_LOAD_ERROR;
+        }
       } else {
         //
         // Set base and end to bypass processing below.
@@ -674,6 +678,10 @@ Returns:
                         ImageContext,
                         RelocDir->VirtualAddress + RelocDir->Size - 1
                         );
+        if (RelocBase == NULL || RelocBaseEnd == NULL || RelocBaseEnd < RelocBase) {
+          ImageContext->ImageError = IMAGE_ERROR_FAILED_RELOCATION;
+          return RETURN_LOAD_ERROR;
+        }
       } else {
         //
         // Set base and end to bypass processing below.
@@ -710,6 +718,10 @@ Returns:
     RelocEnd  = (UINT16 *) ((CHAR8 *) RelocBase + RelocBase->SizeOfBlock);
     if (!(ImageContext->IsTeImage)) {
       FixupBase = PeCoffLoaderImageAddress (ImageContext, RelocBase->VirtualAddress);
+      if (FixupBase == NULL) {
+        ImageContext->ImageError = IMAGE_ERROR_FAILED_RELOCATION;
+        return RETURN_LOAD_ERROR;
+      }
     } else {
       FixupBase = (CHAR8 *)(UINTN)(ImageContext->ImageAddress +
                     RelocBase->VirtualAddress +
diff --git a/BaseTools/Source/C/Common/EfiUtilityMsgs.c b/BaseTools/Source/C/Common/EfiUtilityMsgs.c
index 438f338..7b4c231 100644
--- a/BaseTools/Source/C/Common/EfiUtilityMsgs.c
+++ b/BaseTools/Source/C/Common/EfiUtilityMsgs.c
@@ -1,7 +1,7 @@
 /** @file
 EFI tools utility functions to display warning, error, and informational messages
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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
@@ -451,14 +451,16 @@ Notes:
     //
     time (&CurrentTime);
     NewTime = localtime (&CurrentTime);
-    fprintf (stdout, "%04d-%02d-%02d %02d:%02d:%02d",
-                     NewTime->tm_year + 1900,
-                     NewTime->tm_mon + 1,
-                     NewTime->tm_mday,
-                     NewTime->tm_hour,
-                     NewTime->tm_min,
-                     NewTime->tm_sec
-                     );
+    if (NewTime != NULL) {
+      fprintf (stdout, "%04d-%02d-%02d %02d:%02d:%02d",
+                       NewTime->tm_year + 1900,
+                       NewTime->tm_mon + 1,
+                       NewTime->tm_mday,
+                       NewTime->tm_hour,
+                       NewTime->tm_min,
+                       NewTime->tm_sec
+                       );
+    }
     if (Cptr != NULL) {
       sprintf (Line, ": %s", Cptr);
       if (LineNumber != 0) {
diff --git a/BaseTools/Source/C/Common/FirmwareVolumeBuffer.c b/BaseTools/Source/C/Common/FirmwareVolumeBuffer.c
index 7988d8e..a287fe1 100644
--- a/BaseTools/Source/C/Common/FirmwareVolumeBuffer.c
+++ b/BaseTools/Source/C/Common/FirmwareVolumeBuffer.c
@@ -1,7 +1,7 @@
 /** @file
 EFI Firmware Volume routines which work on a Fv image in buffers.
 
-Copyright (c) 1999 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 1999 - 2016, 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
@@ -353,6 +353,9 @@ Returns:
 
   if (*DestinationFv == NULL) {
     *DestinationFv = CommonLibBinderAllocate (size);
+    if (*DestinationFv == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
   }
 
   CommonLibBinderCopyMem (*DestinationFv, SourceFv, size);
diff --git a/BaseTools/Source/C/Common/MyAlloc.c b/BaseTools/Source/C/Common/MyAlloc.c
index eabba57..be7c515 100644
--- a/BaseTools/Source/C/Common/MyAlloc.c
+++ b/BaseTools/Source/C/Common/MyAlloc.c
@@ -1,7 +1,7 @@
 /** @file
 File for memory allocation tracking functions.
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -73,7 +73,18 @@ MyCheck (
   //
   // Check parameters.
   //
-  if (File == NULL || Line == 0) {
+  if (File == NULL) {
+    printf (
+      "\nMyCheck(Final=%u, File=NULL, Line=%u)"
+      "Invalid parameter(s).\n",
+      Final,
+      (unsigned)Line
+      );
+
+    exit (1);
+  }
+
+  if (Line == 0) {
     printf (
       "\nMyCheck(Final=%u, File=%s, Line=%u)"
       "Invalid parameter(s).\n",
@@ -190,7 +201,18 @@ MyAlloc (
   //
   // Check for invalid parameters.
   //
-  if (Size == 0 || File == NULL || Line == 0) {
+  if (File == NULL) {
+    printf (
+      "\nMyAlloc(Size=%u, File=NULL, Line=%u)"
+      "\nInvalid parameter(s).\n",
+      (unsigned)Size,
+      (unsigned)Line
+      );
+
+    exit (1);
+  }
+
+  if (Size == 0 || Line == 0) {
     printf (
       "\nMyAlloc(Size=%u, File=%s, Line=%u)"
       "\nInvalid parameter(s).\n",
@@ -303,7 +325,19 @@ MyRealloc (
   //
   // Check for invalid parameter(s).
   //
-  if (Size == 0 || File == NULL || Line == 0) {
+  if (File == NULL) {
+    printf (
+      "\nMyRealloc(Ptr=%p, Size=%u, File=NULL, Line=%u)"
+      "\nInvalid parameter(s).\n",
+      Ptr,
+      (unsigned)Size,
+      (unsigned)Line
+      );
+
+    exit (1);
+  }
+
+  if (Size == 0 || Line == 0) {
     printf (
       "\nMyRealloc(Ptr=%p, Size=%u, File=%s, Line=%u)"
       "\nInvalid parameter(s).\n",
@@ -408,7 +442,18 @@ MyFree (
   //
   // Check for invalid parameter(s).
   //
-  if (File == NULL || Line == 0) {
+  if (File == NULL) {
+    printf (
+      "\nMyFree(Ptr=%p, File=NULL, Line=%u)"
+      "\nInvalid parameter(s).\n",
+      Ptr,
+      (unsigned)Line
+      );
+
+    exit (1);
+  }
+
+  if (Line == 0) {
     printf (
       "\nMyFree(Ptr=%p, File=%s, Line=%u)"
       "\nInvalid parameter(s).\n",
diff --git a/BaseTools/Source/C/Common/ParseGuidedSectionTools.c b/BaseTools/Source/C/Common/ParseGuidedSectionTools.c
index e3f0ccb..fc8f488 100644
--- a/BaseTools/Source/C/Common/ParseGuidedSectionTools.c
+++ b/BaseTools/Source/C/Common/ParseGuidedSectionTools.c
@@ -1,7 +1,7 @@
 /** @file
 Helper functions for parsing GuidedSectionTools.txt
 
-Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2007 - 2016, 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        
@@ -144,13 +144,14 @@ Returns:
           NewGuidTool->Name = CloneString(Tool->Strings[1]);
           NewGuidTool->Path = CloneString(Tool->Strings[2]);
           NewGuidTool->Next = NULL;
+
+          if (FirstGuidTool == NULL) {
+            FirstGuidTool = NewGuidTool;
+          } else {
+            LastGuidTool->Next = NewGuidTool;
+          }
+          LastGuidTool = NewGuidTool;
         }
-        if (FirstGuidTool == NULL) {
-          FirstGuidTool = NewGuidTool;
-        } else {
-          LastGuidTool->Next = NewGuidTool;
-        }
-        LastGuidTool = NewGuidTool;
       }
       FreeStringList (Tool);
     }
diff --git a/BaseTools/Source/C/Common/TianoCompress.c b/BaseTools/Source/C/Common/TianoCompress.c
index e5175fc..252b829 100644
--- a/BaseTools/Source/C/Common/TianoCompress.c
+++ b/BaseTools/Source/C/Common/TianoCompress.c
@@ -4,7 +4,7 @@ coding. LZ77 transforms the source data into a sequence of Original Characters
 and Pointers to repeated strings. This sequence is further divided into Blocks 
 and Huffman codings are applied to each Block.
   
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2016, 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        
@@ -417,6 +417,9 @@ Returns:
   UINT32  Index;
 
   mText = malloc (WNDSIZ * 2 + MAXMATCH);
+  if (mText == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
   for (Index = 0; Index < WNDSIZ * 2 + MAXMATCH; Index++) {
     mText[Index] = 0;
   }
@@ -427,6 +430,10 @@ Returns:
   mParent     = malloc (WNDSIZ * 2 * sizeof (*mParent));
   mPrev       = malloc (WNDSIZ * 2 * sizeof (*mPrev));
   mNext       = malloc ((MAX_HASH_VAL + 1) * sizeof (*mNext));
+  if (mLevel == NULL || mChildCount == NULL || mPosition == NULL ||
+    mParent == NULL || mPrev == NULL || mNext == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
 
   mBufSiz     = BLKSIZ;
   mBuf        = malloc (mBufSiz);
-- 
1.9.5.msysgit.0



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

* [PATCH 02/52] BaseTools/EfiRom: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
  2016-10-12 12:19 ` [PATCH 01/52] BaseTools/C/Common: Avoid possible NULL pointer dereference Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 03/52] BaseTools/GenFfs: " Hao Wu
                   ` (50 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/EfiRom/EfiRom.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/BaseTools/Source/C/EfiRom/EfiRom.c b/BaseTools/Source/C/EfiRom/EfiRom.c
index 7a57912..32aa3dc 100644
--- a/BaseTools/Source/C/EfiRom/EfiRom.c
+++ b/BaseTools/Source/C/EfiRom/EfiRom.c
@@ -1,7 +1,7 @@
 /** @file
 Utility program to create an EFI option ROM image from binary and EFI PE32 files.
 
-Copyright (c) 1999 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 1999 - 2016, 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
@@ -95,24 +95,26 @@ Returns:
   // the command line, or the first input filename with a different extension.
   //
   if (!mOptions.OutFileName[0]) {
-    strcpy (mOptions.OutFileName, mOptions.FileList->FileName);
-    //
-    // Find the last . on the line and replace the filename extension with
-    // the default
-    //
-    for (Ext = mOptions.OutFileName + strlen (mOptions.OutFileName) - 1;
-         (Ext >= mOptions.OutFileName) && (*Ext != '.') && (*Ext != '\\');
-         Ext--
-        )
-      ;
-    //
-    // If dot here, then insert extension here, otherwise append
-    //
-    if (*Ext != '.') {
-      Ext = mOptions.OutFileName + strlen (mOptions.OutFileName);
-    }
+    if (mOptions.FileList != NULL) {
+      strcpy (mOptions.OutFileName, mOptions.FileList->FileName);
+      //
+      // Find the last . on the line and replace the filename extension with
+      // the default
+      //
+      for (Ext = mOptions.OutFileName + strlen (mOptions.OutFileName) - 1;
+           (Ext >= mOptions.OutFileName) && (*Ext != '.') && (*Ext != '\\');
+           Ext--
+          )
+        ;
+      //
+      // If dot here, then insert extension here, otherwise append
+      //
+      if (*Ext != '.') {
+        Ext = mOptions.OutFileName + strlen (mOptions.OutFileName);
+      }
 
-    strcpy (Ext, DEFAULT_OUTPUT_EXTENSION);
+      strcpy (Ext, DEFAULT_OUTPUT_EXTENSION);
+    }
   }
   //
   // Make sure we don't have the same filename for input and output files
-- 
1.9.5.msysgit.0



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

* [PATCH 03/52] BaseTools/GenFfs: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
  2016-10-12 12:19 ` [PATCH 01/52] BaseTools/C/Common: Avoid possible NULL pointer dereference Hao Wu
  2016-10-12 12:19 ` [PATCH 02/52] BaseTools/EfiRom: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 04/52] BaseTools/GenFv: " Hao Wu
                   ` (49 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenFfs/GenFfs.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/BaseTools/Source/C/GenFfs/GenFfs.c b/BaseTools/Source/C/GenFfs/GenFfs.c
index 433b608..25a722a 100644
--- a/BaseTools/Source/C/GenFfs/GenFfs.c
+++ b/BaseTools/Source/C/GenFfs/GenFfs.c
@@ -1,7 +1,7 @@
 /** @file
 This file contains functions required to generate a Firmware File System file.
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -842,7 +842,7 @@ Returns:
                );
   }
 
-  if (EFI_ERROR (Status)) {
+  if (EFI_ERROR (Status) || (FileBuffer == NULL)) {
     goto Finish;
   }
   
@@ -915,22 +915,24 @@ Returns:
   //
   // Open output file to write ffs data.
   //
-  remove(OutputFileName);
-  FfsFile = fopen (LongFilePath (OutputFileName), "wb");
-  if (FfsFile == NULL) {
-    Error (NULL, 0, 0001, "Error opening file", OutputFileName);
-    goto Finish;
-  }
-  //
-  // write header
-  //
-  fwrite (&FfsFileHeader, 1, HeaderSize, FfsFile);
-  //
-  // write data
-  //
-  fwrite (FileBuffer, 1, FileSize - HeaderSize, FfsFile);
+  if (OutputFileName != NULL) {
+    remove(OutputFileName);
+    FfsFile = fopen (LongFilePath (OutputFileName), "wb");
+    if (FfsFile == NULL) {
+      Error (NULL, 0, 0001, "Error opening file", OutputFileName);
+      goto Finish;
+    }
+    //
+    // write header
+    //
+    fwrite (&FfsFileHeader, 1, HeaderSize, FfsFile);
+    //
+    // write data
+    //
+    fwrite (FileBuffer, 1, FileSize - HeaderSize, FfsFile);
 
-  fclose (FfsFile);
+    fclose (FfsFile);
+  }
 
 Finish:
   if (InputFileName != NULL) {
-- 
1.9.5.msysgit.0



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

* [PATCH 04/52] BaseTools/GenFv: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (2 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 03/52] BaseTools/GenFfs: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 05/52] BaseTools/GenFw: " Hao Wu
                   ` (48 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index fab7c94..f7e3ba5 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -1,7 +1,7 @@
 /** @file
 This file contains the internal functions required to generate a Firmware Volume.
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
 Portions Copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.<BR>
 Portions Copyright (c) 2016 HP Development Company, L.P.<BR>
 This program and the accompanying materials                          
@@ -2494,6 +2494,10 @@ Returns:
     // Open the FV Extension Header file
     //
     FvExtHeaderFile = fopen (LongFilePath (mFvDataInfo.FvExtHeaderFile), "rb");
+    if (FvExtHeaderFile == NULL) {
+      Error (NULL, 0, 0001, "Error opening file", mFvDataInfo.FvExtHeaderFile);
+      return EFI_ABORTED;
+    }
 
     //
     // Get the file size
-- 
1.9.5.msysgit.0



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

* [PATCH 05/52] BaseTools/GenFw: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (3 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 04/52] BaseTools/GenFv: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 06/52] BaseTools/GenPage: " Hao Wu
                   ` (47 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenFw/Elf32Convert.c |  8 ++++++++
 BaseTools/Source/C/GenFw/Elf64Convert.c | 10 +++++++++-
 BaseTools/Source/C/GenFw/ElfConvert.c   |  7 ++++++-
 BaseTools/Source/C/GenFw/GenFw.c        | 18 ++++++++++++++++--
 4 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
index 8fca7fb..f420bc8 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -167,6 +167,10 @@ InitializeElf32 (
   // Create COFF Section offset buffer and zero.
   //
   mCoffSectionsOffset = (UINT32 *)malloc(mEhdr->e_shnum * sizeof (UINT32));
+  if (mCoffSectionsOffset == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return FALSE;
+  }
   memset(mCoffSectionsOffset, 0, mEhdr->e_shnum * sizeof(UINT32));
 
   //
@@ -526,6 +530,10 @@ ScanSections32 (
   // Allocate base Coff file.  Will be expanded later for relocations.
   //
   mCoffFile = (UINT8 *)malloc(mCoffOffset);
+  if (mCoffFile == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+  }
+  assert (mCoffFile != NULL);
   memset(mCoffFile, 0, mCoffOffset);
 
   //
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 9b409b6..acf0216 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1,7 +1,7 @@
 /** @file
 Elf64 convert solution
 
-Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
 Portions copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
 
 This program and the accompanying materials are licensed and made available
@@ -172,6 +172,10 @@ InitializeElf64 (
   //
   VerboseMsg ("Create COFF Section Offset Buffer");
   mCoffSectionsOffset = (UINT32 *)malloc(mEhdr->e_shnum * sizeof (UINT32));
+  if (mCoffSectionsOffset == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return FALSE;
+  }
   memset(mCoffSectionsOffset, 0, mEhdr->e_shnum * sizeof(UINT32));
 
   //
@@ -518,6 +522,10 @@ ScanSections64 (
   // Allocate base Coff file.  Will be expanded later for relocations.
   //
   mCoffFile = (UINT8 *)malloc(mCoffOffset);
+  if (mCoffFile == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+  }
+  assert (mCoffFile != NULL);
   memset(mCoffFile, 0, mCoffOffset);
 
   //
diff --git a/BaseTools/Source/C/GenFw/ElfConvert.c b/BaseTools/Source/C/GenFw/ElfConvert.c
index 6211389..17913ff 100644
--- a/BaseTools/Source/C/GenFw/ElfConvert.c
+++ b/BaseTools/Source/C/GenFw/ElfConvert.c
@@ -1,7 +1,7 @@
 /** @file
 Elf convert solution
 
-Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2016, 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 
@@ -24,6 +24,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <string.h>
 #include <time.h>
 #include <ctype.h>
+#include <assert.h>
 
 #include <Common/UefiBaseTypes.h>
 #include <IndustryStandard/PeImage.h>
@@ -98,6 +99,10 @@ CoffAddFixup(
       mCoffFile,
       mCoffOffset + sizeof(EFI_IMAGE_BASE_RELOCATION) + 2 * MAX_COFF_ALIGNMENT
       );
+    if (mCoffFile == NULL) {
+      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    }
+    assert (mCoffFile != NULL);
     memset (
       mCoffFile + mCoffOffset, 0,
       sizeof(EFI_IMAGE_BASE_RELOCATION) + 2 * MAX_COFF_ALIGNMENT
diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 03bfaa1..c0d1e6a 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -625,6 +625,10 @@ PeCoffConvertImageToXip (
   // Allocate the extra space that we need to grow the image
   //
   XipFile = malloc (XipLength);
+  if (XipFile == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return;
+  }
   memset (XipFile, 0, XipLength);
 
   //
@@ -701,6 +705,10 @@ Returns:
                           + 3 * (sizeof (UINT16) + 3 * sizeof (CHAR16)) 
                           + sizeof (EFI_IMAGE_RESOURCE_DATA_ENTRY);
   HiiSectionHeader = malloc (HiiSectionHeaderSize);
+  if (HiiSectionHeader == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return NULL;
+  }
   memset (HiiSectionHeader, 0, HiiSectionHeaderSize);
 
   HiiSectionOffset = 0;
@@ -1693,6 +1701,10 @@ Returns:
       // Create the resource section header
       //
       HiiSectionHeader = CreateHiiResouceSectionHeader (&HiiSectionHeaderSize, HiiPackageListHeader.PackageLength);
+      if (HiiSectionHeader == NULL) {
+        free (HiiPackageListBuffer);
+        goto Finish;
+      }
       //
       // Wrtie section header and HiiData into File.
       //
@@ -3028,8 +3040,10 @@ Returns:
   }
 
   ptime = localtime (&newtime);
-  DebugMsg (NULL, 0, 9, "New Image Time Stamp", "%04d-%02d-%02d %02d:%02d:%02d",
-            ptime->tm_year + 1900, ptime->tm_mon + 1, ptime->tm_mday, ptime->tm_hour, ptime->tm_min, ptime->tm_sec);
+  if (ptime != NULL) {
+    DebugMsg (NULL, 0, 9, "New Image Time Stamp", "%04d-%02d-%02d %02d:%02d:%02d",
+              ptime->tm_year + 1900, ptime->tm_mon + 1, ptime->tm_mday, ptime->tm_hour, ptime->tm_min, ptime->tm_sec);
+  }
   //
   // Set new time and data into PeImage.
   //
-- 
1.9.5.msysgit.0



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

* [PATCH 06/52] BaseTools/GenPage: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (4 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 05/52] BaseTools/GenFw: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 07/52] BaseTools/GenSec: " Hao Wu
                   ` (46 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenPage/GenPage.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenPage/GenPage.c b/BaseTools/Source/C/GenPage/GenPage.c
index fb829e6..5bf8bf8 100644
--- a/BaseTools/Source/C/GenPage/GenPage.c
+++ b/BaseTools/Source/C/GenPage/GenPage.c
@@ -15,7 +15,7 @@
                           Directory-Ptr Directory {512}
                         ) {4}
 
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2016, 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        
@@ -146,6 +146,10 @@ Return:
   X64_PAGE_TABLE_ENTRY_2M                       *PageDirectoryEntry2MB;
 
   PageTable = (void *)malloc (EFI_PAGE_NUMBER * EFI_SIZE_OF_PAGE);
+  if (PageTable == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return NULL;
+  }
   memset (PageTable, 0, (EFI_PAGE_NUMBER * EFI_SIZE_OF_PAGE));
   PageTablePtr = PageTable;
 
@@ -417,6 +421,10 @@ main (
   // Create X64 page table
   //
   BaseMemory = CreateIdentityMappingPageTables ();
+  if (BaseMemory == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return STATUS_ERROR;
+  }
 
   //
   // Add page table to binary file
-- 
1.9.5.msysgit.0



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

* [PATCH 07/52] BaseTools/GenSec: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (5 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 06/52] BaseTools/GenPage: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 08/52] BaseTools/GenVtf: " Hao Wu
                   ` (45 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenSec/GenSec.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenSec/GenSec.c b/BaseTools/Source/C/GenSec/GenSec.c
index 20b2a10..583ff1d 100644
--- a/BaseTools/Source/C/GenSec/GenSec.c
+++ b/BaseTools/Source/C/GenSec/GenSec.c
@@ -1,7 +1,7 @@
 /** @file
 Creates output file that is a properly formed section per the PI spec.
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -667,6 +667,10 @@ Returns:
     return Status;
   }
 
+  if (FileBuffer == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
   CompressFunction = NULL;
 
   //
@@ -731,6 +735,10 @@ Returns:
 
       return Status;
     }
+
+    if (FileBuffer == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
   }
 
   DebugMsg (NULL, 0, 9, "comprss file size", 
@@ -889,6 +897,10 @@ Returns:
     return Status;
   }
 
+  if (FileBuffer == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
   if (InputLength == 0) {
     Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s can't be zero", InputFileName);
     return EFI_NOT_FOUND;
@@ -1365,7 +1377,9 @@ Returns:
   //
   // GuidValue is only required by Guided section.
   //
-  if ((SectType != EFI_SECTION_GUID_DEFINED) && (CompareGuid (&VendorGuid, &mZeroGuid) != 0)) {
+  if ((SectType != EFI_SECTION_GUID_DEFINED) &&
+    (SectionName != NULL) &&
+    (CompareGuid (&VendorGuid, &mZeroGuid) != 0)) {
     fprintf (stdout, "Warning: the input guid value is not required for this section type %s\n", SectionName);
   }
   
-- 
1.9.5.msysgit.0



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

* [PATCH 08/52] BaseTools/GenVtf: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (6 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 07/52] BaseTools/GenSec: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 09/52] BaseTools/TianoCompress: " Hao Wu
                   ` (44 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenVtf/GenVtf.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/BaseTools/Source/C/GenVtf/GenVtf.c b/BaseTools/Source/C/GenVtf/GenVtf.c
index f6765dd..b68d86a 100644
--- a/BaseTools/Source/C/GenVtf/GenVtf.c
+++ b/BaseTools/Source/C/GenVtf/GenVtf.c
@@ -1125,6 +1125,7 @@ Returns:
   EFI_ABORTED      - Aborted due to one of the many reasons like:
                       (a) Component Size greater than the specified size.
                       (b) Error opening files.
+                      (c) Fail to get the FIT table address.
 
   EFI_INVALID_PARAMETER     Value returned from call to UpdateEntryPoint()
   EFI_OUT_OF_RESOURCES      Memory allocation failure.
@@ -1240,6 +1241,10 @@ Returns:
   }
 
   GetNextAvailableFitPtr (&CompFitPtr);
+  if (CompFitPtr == NULL) {
+    free (Buffer);
+    return EFI_ABORTED;
+  }
 
   CompFitPtr->CompAddress = CompStartAddress | IPF_CACHE_BIT;
   if ((FileSize % 16) != 0) {
@@ -2652,6 +2657,7 @@ Returns:
     }
     SymFileName = VTF_SYM_FILE;
   } else {
+    assert (OutFileName1);
     INTN OutFileNameLen = strlen(OutFileName1);
     INTN NewIndex;
 
@@ -2665,6 +2671,10 @@ Returns:
     } else {
       INTN SymFileNameLen = NewIndex + 1 + strlen(VTF_SYM_FILE);
       SymFileName = malloc(SymFileNameLen + 1);
+      if (SymFileName == NULL) {
+        Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+        goto ERROR;
+      }
       memcpy(SymFileName, OutFileName1, NewIndex + 1);
       memcpy(SymFileName + NewIndex + 1, VTF_SYM_FILE, strlen(VTF_SYM_FILE));
       SymFileName[SymFileNameLen] = '\0';
-- 
1.9.5.msysgit.0



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

* [PATCH 09/52] BaseTools/TianoCompress: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (7 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 08/52] BaseTools/GenVtf: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 10/52] BaseTools/VfrCompile: " Hao Wu
                   ` (43 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 37 ++++++++++++++++--------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index 70f1b61..57253cc 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -5,7 +5,7 @@ and Pointers to repeated strings.
 This sequence is further divided into Blocks and Huffman codings are applied to 
 each Block.
   
-Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2007 - 2016, 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        
@@ -240,6 +240,10 @@ Returns:
   UINT32  Index;
 
   mText = malloc (WNDSIZ * 2 + MAXMATCH);
+  if (mText == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return EFI_OUT_OF_RESOURCES;
+  }
   for (Index = 0; Index < WNDSIZ * 2 + MAXMATCH; Index++) {
     mText[Index] = 0;
   }
@@ -250,6 +254,11 @@ Returns:
   mParent     = malloc (WNDSIZ * 2 * sizeof (*mParent));
   mPrev       = malloc (WNDSIZ * 2 * sizeof (*mPrev));
   mNext       = malloc ((MAX_HASH_VAL + 1) * sizeof (*mNext));
+  if (mLevel == NULL || mChildCount == NULL || mPosition == NULL ||
+    mParent == NULL || mPrev == NULL || mNext == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return EFI_OUT_OF_RESOURCES;
+  }
 
   mBufSiz     = BLKSIZ;
   mBuf        = malloc (mBufSiz);
@@ -1911,20 +1920,18 @@ Returns:
     free(FileBuffer);
     return 1;
   }
-   
-  if (OutputFileName != NULL) {
-    OutputFile = fopen (LongFilePath (OutputFileName), "wb");
-    if (OutputFile == NULL) {
-      Error (NULL, 0, 0001, "Error opening output file for writing", OutputFileName);
+
+  if (OutputFileName == NULL) {
+    OutputFileName = DEFAULT_OUTPUT_FILE;
+  }
+  OutputFile = fopen (LongFilePath (OutputFileName), "wb");
+  if (OutputFile == NULL) {
+    Error (NULL, 0, 0001, "Error opening output file for writing", OutputFileName);
     if (InputFile != NULL) {
       fclose (InputFile);
-      }
-      goto ERROR;
-      }
-    } else {
-      OutputFileName = DEFAULT_OUTPUT_FILE;
-      OutputFile = fopen (LongFilePath (OutputFileName), "wb");
     }
+    goto ERROR;
+  }
     
   if (ENCODE) {
   //
@@ -1942,12 +1949,18 @@ Returns:
       goto ERROR;
     }
   }
+
   Status = TianoCompress ((UINT8 *)FileBuffer, InputLength, OutBuffer, &DstSize);
   if (Status != EFI_SUCCESS) {
     Error (NULL, 0, 0007, "Error compressing file", NULL);
     goto ERROR;
   }
 
+  if (OutBuffer == NULL) {
+    Error (NULL, 0, 4001, "Resource:", "Memory cannot be allocated!");
+    goto ERROR;
+  }
+
   fwrite(OutBuffer,(size_t)DstSize, 1, OutputFile);
   free(Scratch);
   free(FileBuffer);
-- 
1.9.5.msysgit.0



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

* [PATCH 10/52] BaseTools/VfrCompile: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (8 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 09/52] BaseTools/TianoCompress: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 11/52] BaseTools/VolInfo: " Hao Wu
                   ` (42 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp    | 16 +++++++++++++++-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp |  2 +-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
index 0b7b8b1..aa27ce0 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
@@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 **/
 
 #include "stdio.h"
+#include "assert.h"
 #include "VfrFormPkg.h"
 
 /*
@@ -669,6 +670,8 @@ CFormPkg::AdjustDynamicInsertOpcode (
 
   InserPositionNode  = GetBinBufferNodeForAddr(InserPositionAddr);
   InsertOpcodeNode = GetBinBufferNodeForAddr(InsertOpcodeAddr);
+  assert (InserPositionNode != NULL);
+  assert (InsertOpcodeNode  != NULL);
 
   if (InserPositionNode == InsertOpcodeNode) {
     //
@@ -741,6 +744,8 @@ CFormPkg::AdjustDynamicInsertOpcode (
       // Insert the last restore data node.
       //
       TmpNode = GetNodeBefore (InsertOpcodeNode);
+      assert (TmpNode != NULL);
+
       if (TmpNode == InserPositionNode) {
         NewRestoreNodeBegin->mNext = NewRestoreNodeEnd;
       } else {
@@ -790,6 +795,8 @@ CFormPkg::AdjustDynamicInsertOpcode (
       mBufferNodeQueueTail = NewLastEndNode;
     } else if (mBufferNodeQueueTail->mBufferFree - mBufferNodeQueueTail->mBufferStart == 2) {
       TmpNode = GetNodeBefore(mBufferNodeQueueTail);
+      assert (TmpNode != NULL);
+
       TmpNode->mNext = NewRestoreNodeBegin;
       if (NewRestoreNodeEnd != NULL) {
         NewRestoreNodeEnd->mNext = mBufferNodeQueueTail;
@@ -1314,7 +1321,7 @@ CIfrRecordInfoDB::IfrAdjustDynamicOpcodeInRecords (
   //
   // Check the nodes whether exist.
   //
-  if (pNodeBeforeDynamic == NULL || pAdjustNode == NULL) {
+  if (pNodeBeforeDynamic == NULL || pAdjustNode == NULL || pNodeBeforeAdjust == NULL) {
     return FALSE;
   }
 
@@ -1854,6 +1861,10 @@ CIfrRecordInfoDB::IfrCreateDefaultForQuestion (
       pSNode = pSNode->mNext;
       OpcodeCount++;
     }
+
+    assert (pSNode);
+    assert (pENode);
+
     //
     // Record the offset of node which need to be adjust, will move the new created default opcode to this offset.
     //
@@ -1875,6 +1886,7 @@ CIfrRecordInfoDB::IfrCreateDefaultForQuestion (
         while (pSNode != NULL && pSNode->mNext != NULL && OpcodeNumber-- != 0) {
           pOpHead = (EFI_IFR_OP_HEADER *) pSNode->mIfrBinBuf;
           Obj = new CIfrObj (pOpHead->OpCode, NULL, pSNode->mBinBufLen, FALSE);
+          assert (Obj != NULL);
           Obj->SetLineNo (pSNode->mLineNo);
           ObjBinBuf = Obj->GetObjBinAddr();
           memcpy (ObjBinBuf, pSNode->mIfrBinBuf, (UINTN)pSNode->mBinBufLen);
@@ -2378,6 +2390,8 @@ CIfrObj::CIfrObj (
   mObjBinBuf   = ((DelayEmit == FALSE) && (gCreateOp == TRUE)) ? gCFormPkg.IfrBinBufferGet (mObjBinLen) : new CHAR8[EFI_IFR_MAX_LENGTH];
   mRecordIdx   = (gCreateOp == TRUE) ? gCIfrRecordInfoDB.IfrRecordRegister (0xFFFFFFFF, mObjBinBuf, mObjBinLen, mPkgOffset) : EFI_IFR_RECORDINFO_IDX_INVALUD;
 
+  assert (mObjBinBuf != NULL);
+
   if (IfrObj != NULL) {
     *IfrObj    = mObjBinBuf;
   }
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index b3d1ac5..d2cb5cc 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -665,7 +665,7 @@ CVfrVarDataTypeDB::GetTypeField (
 {
   SVfrDataField  *pField = NULL;
 
-  if ((FName == NULL) && (Type == NULL)) {
+  if ((FName == NULL) || (Type == NULL)) {
     return VFR_RETURN_FATAL_ERROR;
   }
 
-- 
1.9.5.msysgit.0



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

* [PATCH 11/52] BaseTools/VolInfo: Avoid possible NULL pointer dereference
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (9 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 10/52] BaseTools/VfrCompile: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 12/52] BaseTools/TianoCompress: Initialize local variables before being used Hao Wu
                   ` (41 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VolInfo/VolInfo.c | 46 ++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c b/BaseTools/Source/C/VolInfo/VolInfo.c
index 3a2686a..1ea2f49 100644
--- a/BaseTools/Source/C/VolInfo/VolInfo.c
+++ b/BaseTools/Source/C/VolInfo/VolInfo.c
@@ -265,6 +265,10 @@ Returns:
         OpenSslPath = OpenSslCommand;
       } else {
         OpenSslPath = malloc(strlen(OpenSslEnv)+strlen(OpenSslCommand)+1);
+        if (OpenSslPath == NULL) {
+          Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+          return GetUtilityStatus ();
+        }
         CombinePath(OpenSslEnv, OpenSslCommand, OpenSslPath);
       }
       if (OpenSslPath == NULL){
@@ -1623,9 +1627,11 @@ Returns:
     SectionHeaderLen = GetSectionHeaderLength((EFI_COMMON_SECTION_HEADER *)Ptr);
 
     SectionName = SectionNameToStr (Type);
-    printf ("------------------------------------------------------------\n");
-    printf ("  Type:  %s\n  Size:  0x%08X\n", SectionName, (unsigned) SectionLength);
-    free (SectionName);
+    if (SectionName != NULL) {
+      printf ("------------------------------------------------------------\n");
+      printf ("  Type:  %s\n  Size:  0x%08X\n", SectionName, (unsigned) SectionLength);
+      free (SectionName);
+    }
 
     switch (Type) {
     case EFI_SECTION_RAW:
@@ -1653,6 +1659,10 @@ Returns:
           strlen (ToolOutputFileName) +
           1
           );
+        if (SystemCommand == NULL) {
+          Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+          return EFI_OUT_OF_RESOURCES;
+        }
         sprintf (
           SystemCommand,
           SystemCommandFormatString,
@@ -1678,12 +1688,18 @@ Returns:
             nFileLen = ftell(fp);
             fseek(fp,0,SEEK_SET);
             StrLine = malloc(nFileLen);
+            if (StrLine == NULL) {
+              fclose(fp);
+              free (SystemCommand);
+              Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+              return EFI_OUT_OF_RESOURCES;
+            }
             fgets(StrLine, nFileLen, fp);
             NewStr = strrchr (StrLine, '=');
             printf ("  SHA1: %s\n", NewStr + 1);
             free (StrLine);
+            fclose(fp);
           }
-          fclose(fp);
         }
         remove(ToolInputFileName);
         remove(ToolOutputFileName);
@@ -1845,6 +1861,19 @@ Returns:
         close(fd2);
        #endif
 
+        if ((ToolInputFile == NULL) || (ToolOutputFile == NULL)) {
+          if (ToolInputFile != NULL) {
+            free (ToolInputFile);
+          }
+          if (ToolOutputFile != NULL) {
+            free (ToolOutputFile);
+          }
+          free (ExtractionTool);
+
+          Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+          return EFI_OUT_OF_RESOURCES;
+        }
+
         //
         // Construction 'system' command string
         //
@@ -1856,6 +1885,14 @@ Returns:
           strlen (ToolOutputFile) +
           1
           );
+        if (SystemCommand == NULL) {
+          free (ToolInputFile);
+          free (ToolOutputFile);
+          free (ExtractionTool);
+
+          Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+          return EFI_OUT_OF_RESOURCES;
+        }
         sprintf (
           SystemCommand,
           SystemCommandFormatString,
@@ -1884,6 +1921,7 @@ Returns:
             );
         remove (ToolOutputFile);
         free (ToolOutputFile);
+        free (SystemCommand);
         if (EFI_ERROR (Status)) {
           Error (NULL, 0, 0004, "unable to read decoded GUIDED section", NULL);
           return EFI_SECTION_ERROR;
-- 
1.9.5.msysgit.0



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

* [PATCH 12/52] BaseTools/TianoCompress: Initialize local variables before being used
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (10 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 11/52] BaseTools/VolInfo: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 13/52] BaseTools/VfrCompile: " Hao Wu
                   ` (40 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index 57253cc..b994d93 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -2153,7 +2153,7 @@ Returns:
   UINT16  Start[18];
   UINT16  *Pointer;
   UINT16  Index3;
-  volatile UINT16  Index;
+  UINT16  Index;
   UINT16  Len;
   UINT16  Char;
   UINT16  JuBits;
@@ -2163,7 +2163,7 @@ Returns:
   UINT16  WordOfStart;
   UINT16  WordOfCount;
 
-  for (Index = 1; Index <= 16; Index++) {
+  for (Index = 0; Index <= 16; Index++) {
     Count[Index] = 0;
   }
 
@@ -2171,6 +2171,7 @@ Returns:
     Count[BitLen[Index]]++;
   }
 
+  Start[0] = 0;
   Start[1] = 0;
 
   for (Index = 1; Index <= 16; Index++) {
@@ -2188,6 +2189,7 @@ Returns:
 
   JuBits = (UINT16) (16 - TableBits);
 
+  Weight[0] = 0;
   for (Index = 1; Index <= TableBits; Index++) {
     Start[Index] >>= JuBits;
     Weight[Index] = (UINT16) (1U << (TableBits - Index));
-- 
1.9.5.msysgit.0



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

* [PATCH 13/52] BaseTools/VfrCompile: Initialize local variables before being used
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (11 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 12/52] BaseTools/TianoCompress: Initialize local variables before being used Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 14/52] BaseTools/GenBootSector: Fix parameter format mismatch in printf functions Hao Wu
                   ` (39 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp    | 4 +++-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
index aa27ce0..124b8e8 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
@@ -825,7 +825,7 @@ CFormPkg::DeclarePendingQuestion (
   CHAR8          FName[MAX_NAME_LEN];
   CHAR8          *SName;
   CHAR8          *NewStr;
-  UINT32         ShrinkSize;
+  UINT32         ShrinkSize = 0;
   EFI_VFR_RETURN_CODE  ReturnCode;
   EFI_VFR_VARSTORE_TYPE VarStoreType  = EFI_VFR_VARSTORE_INVALID;
   EFI_VARSTORE_ID       VarStoreId    = EFI_VARSTORE_ID_INVALID;
@@ -1297,6 +1297,7 @@ CIfrRecordInfoDB::IfrAdjustDynamicOpcodeInRecords (
   SIfrRecord         *pAdjustNode, *pNodeBeforeAdjust;
   SIfrRecord         *pNodeBeforeDynamic;
 
+  pPreNode            = NULL;
   pAdjustNode         = NULL;
   pNodeBeforeDynamic  = NULL;
   OpcodeOffset        = 0;
@@ -1845,6 +1846,7 @@ CIfrRecordInfoDB::IfrCreateDefaultForQuestion (
     // Point to the first expression opcode.
     //
     pSNode = pDefaultNode->mNext;
+    pENode = NULL;
     ScopeCount++;
     //
     // Get opcode number behind the EFI_IFR_DEFAULT_2 until reach its END opcode (including the END opcode of EFI_IFR_DEFAULT_2)
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index d2cb5cc..1ab95be 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -1328,7 +1328,7 @@ SVfrVarStorageNode::SVfrVarStorageNode (
   if (Guid != NULL) {
     mGuid = *Guid;
   } else {
-    memset (&Guid, 0, sizeof (EFI_GUID));
+    memset (&mGuid, 0, sizeof (EFI_GUID));
   }
   if (StoreName != NULL) {
     mVarStoreName = new CHAR8[strlen(StoreName) + 1];
@@ -1355,7 +1355,7 @@ SVfrVarStorageNode::SVfrVarStorageNode (
   if (Guid != NULL) {
     mGuid = *Guid;
   } else {
-    memset (&Guid, 0, sizeof (EFI_GUID));
+    memset (&mGuid, 0, sizeof (EFI_GUID));
   }
   if (StoreName != NULL) {
     mVarStoreName = new CHAR8[strlen(StoreName) + 1];
-- 
1.9.5.msysgit.0



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

* [PATCH 14/52] BaseTools/GenBootSector: Fix parameter format mismatch in printf functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (12 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 13/52] BaseTools/VfrCompile: " Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:19 ` [PATCH 15/52] BaseTools/VolInfo: " Hao Wu
                   ` (38 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenBootSector/GenBootSector.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenBootSector/GenBootSector.c b/BaseTools/Source/C/GenBootSector/GenBootSector.c
index c218548..efdfcd2 100644
--- a/BaseTools/Source/C/GenBootSector/GenBootSector.c
+++ b/BaseTools/Source/C/GenBootSector/GenBootSector.c
@@ -4,7 +4,7 @@ Reading/writing MBR/DBR.
     If we write MBR to disk, we just update the MBR code and the partition table wouldn't be over written.
     If we process DBR, we will patch MBR to set first partition active if no active partition exists.
     
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2016, 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        
@@ -169,7 +169,7 @@ Return:
       stderr, 
       "error E0005: CreateFile failed: Volume = %s, LastError = 0x%x\n", 
       VolumeAccessPath, 
-      GetLastError ()
+      (unsigned) GetLastError ()
       );
     return FALSE;
   }
@@ -599,7 +599,7 @@ GetPathInfo (
     }
 
     if (!GetDriveInfo(VolumeLetter, &DriveInfo)) {
-      fprintf (stderr, "ERROR: GetDriveInfo - 0x%x\n", GetLastError ());
+      fprintf (stderr, "ERROR: GetDriveInfo - 0x%x\n", (unsigned) GetLastError ());
       return ErrorPath;
     }
 
@@ -792,7 +792,7 @@ main (
       (OutputPathInfo.Type != PathFile) ? "Write" : "Read", 
       ProcessMbr ? "MBR" : "DBR", 
       ErrorStatusDesc[Status],
-      GetLastError ()
+      (unsigned) GetLastError ()
       );
     return 1;
   }
-- 
1.9.5.msysgit.0



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

* [PATCH 15/52] BaseTools/VolInfo: Fix parameter format mismatch in printf functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (13 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 14/52] BaseTools/GenBootSector: Fix parameter format mismatch in printf functions Hao Wu
@ 2016-10-12 12:19 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 16/52] BaseTools/C/Common: Fix parameter format mismatch in scanf functions Hao Wu
                   ` (37 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:19 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VolInfo/VolInfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c b/BaseTools/Source/C/VolInfo/VolInfo.c
index 1ea2f49..5da6582 100644
--- a/BaseTools/Source/C/VolInfo/VolInfo.c
+++ b/BaseTools/Source/C/VolInfo/VolInfo.c
@@ -1708,7 +1708,7 @@ Returns:
       break;
 
     case EFI_SECTION_USER_INTERFACE:
-      printf ("  String: %ls\n", (CHAR16 *) &((EFI_USER_INTERFACE_SECTION *) Ptr)->FileNameString);
+      printf ("  String: %ls\n", (wchar_t *) &((EFI_USER_INTERFACE_SECTION *) Ptr)->FileNameString);
       break;
 
     case EFI_SECTION_FIRMWARE_VOLUME_IMAGE:
-- 
1.9.5.msysgit.0



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

* [PATCH 16/52] BaseTools/C/Common: Fix parameter format mismatch in scanf functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (14 preceding siblings ...)
  2016-10-12 12:19 ` [PATCH 15/52] BaseTools/VolInfo: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 17/52] BaseTools/GenFv: " Hao Wu
                   ` (36 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/Common/ParseInf.c          | 24 ++++++++++++------------
 BaseTools/Source/C/Common/SimpleFileParsing.c | 14 +++++++-------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/BaseTools/Source/C/Common/ParseInf.c b/BaseTools/Source/C/Common/ParseInf.c
index 9e85a88..28ed1ca 100644
--- a/BaseTools/Source/C/Common/ParseInf.c
+++ b/BaseTools/Source/C/Common/ParseInf.c
@@ -1,7 +1,7 @@
 /** @file
 This contains some useful functions for parsing INF files.
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -429,17 +429,17 @@ Returns:
   Index = sscanf (
             AsciiGuidBuffer,
             "%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
-            &Data1,
-            &Data2,
-            &Data3,
-            &Data4[0],
-            &Data4[1],
-            &Data4[2],
-            &Data4[3],
-            &Data4[4],
-            &Data4[5],
-            &Data4[6],
-            &Data4[7]
+            (INT32 *) &Data1,
+            (INT32 *) &Data2,
+            (INT32 *) &Data3,
+            (INT32 *) &Data4[0],
+            (INT32 *) &Data4[1],
+            (INT32 *) &Data4[2],
+            (INT32 *) &Data4[3],
+            (INT32 *) &Data4[4],
+            (INT32 *) &Data4[5],
+            (INT32 *) &Data4[6],
+            (INT32 *) &Data4[7]
             );
 
   //
diff --git a/BaseTools/Source/C/Common/SimpleFileParsing.c b/BaseTools/Source/C/Common/SimpleFileParsing.c
index 877bb6f..a99a0ea 100644
--- a/BaseTools/Source/C/Common/SimpleFileParsing.c
+++ b/BaseTools/Source/C/Common/SimpleFileParsing.c
@@ -1,7 +1,7 @@
 /** @file
 Generic but simple file parsing routines.
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -547,7 +547,7 @@ Returns:
       }
 
       mGlobals.SourceFile.FileBufferPtr += 2;
-      sscanf (mGlobals.SourceFile.FileBufferPtr, "%x", &Val);
+      sscanf (mGlobals.SourceFile.FileBufferPtr, "%x", (INT32 *) &Val);
       *Value = (UINT32) Val;
       while (isxdigit ((int)mGlobals.SourceFile.FileBufferPtr[0])) {
         mGlobals.SourceFile.FileBufferPtr++;
@@ -1305,7 +1305,7 @@ Returns:
       goto Done;
     }
 
-    sscanf (TempString, "%x", &Value32);
+    sscanf (TempString, "%x", (INT32 *) &Value32);
     Value->Data1 = Value32;
     //
     // Next two UINT16 fields
@@ -1320,7 +1320,7 @@ Returns:
       goto Done;
     }
 
-    sscanf (TempString, "%x", &Value32);
+    sscanf (TempString, "%x", (INT32 *) &Value32);
     Value->Data2 = (UINT16) Value32;
 
     if (mGlobals.SourceFile.FileBufferPtr[0] != '-') {
@@ -1333,7 +1333,7 @@ Returns:
       goto Done;
     }
 
-    sscanf (TempString, "%x", &Value32);
+    sscanf (TempString, "%x", (INT32 *) &Value32);
     Value->Data3 = (UINT16) Value32;
     //
     // Parse the "AAAA" as two bytes
@@ -1348,7 +1348,7 @@ Returns:
       goto Done;
     }
 
-    sscanf (TempString, "%x", &Value32);
+    sscanf (TempString, "%x", (INT32 *) &Value32);
     Value->Data4[0] = (UINT8) (Value32 >> 8);
     Value->Data4[1] = (UINT8) Value32;
     if (mGlobals.SourceFile.FileBufferPtr[0] != '-') {
@@ -1393,7 +1393,7 @@ Returns:
       //
       TempString2[0]  = TempString[Index * 2];
       TempString2[1]  = TempString[Index * 2 + 1];
-      sscanf (TempString2, "%x", &Value32);
+      sscanf (TempString2, "%x", (INT32 *) &Value32);
       Value->Data4[Index + 2] = (UINT8) Value32;
     }
 
-- 
1.9.5.msysgit.0



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

* [PATCH 17/52] BaseTools/GenFv: Fix parameter format mismatch in scanf functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (15 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 16/52] BaseTools/C/Common: Fix parameter format mismatch in scanf functions Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 18/52] BaseTools/GenFw: " Hao Wu
                   ` (35 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index f7e3ba5..8c769b4 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -908,7 +908,7 @@ Returns:
         FunctionType = 2;
         fgets (Line, MAX_LINE_LEN, PeMapFile);
       } else if (stricmp (KeyWord, "Preferred") ==0) {
-        sscanf (Line + strlen (" Preferred load address is"), "%llx", &TempLongAddress);
+        sscanf (Line + strlen (" Preferred load address is"), "%llx", (long long *) &TempLongAddress);
         LinkTimeBaseAddress = (UINT64) TempLongAddress;
       }
       continue;
@@ -917,14 +917,14 @@ Returns:
     // Printf Function Information
     //
     if (FunctionType == 1) {
-      sscanf (Line, "%s %s %llx %s", KeyWord, FunctionName, &TempLongAddress, FunctionTypeName);
+      sscanf (Line, "%s %s %llx %s", KeyWord, FunctionName, (long long *) &TempLongAddress, FunctionTypeName);
       FunctionAddress = (UINT64) TempLongAddress;
       if (FunctionTypeName [1] == '\0' && (FunctionTypeName [0] == 'f' || FunctionTypeName [0] == 'F')) {
         fprintf (FvMapFile, "  0x%010llx    ", (unsigned long long) (ImageBaseAddress + FunctionAddress - LinkTimeBaseAddress));
         fprintf (FvMapFile, "%s\n", FunctionName);
       }
     } else if (FunctionType == 2) {
-      sscanf (Line, "%s %s %llx %s", KeyWord, FunctionName, &TempLongAddress, FunctionTypeName);
+      sscanf (Line, "%s %s %llx %s", KeyWord, FunctionName, (long long *) &TempLongAddress, FunctionTypeName);
       FunctionAddress = (UINT64) TempLongAddress;
       if (FunctionTypeName [1] == '\0' && (FunctionTypeName [0] == 'f' || FunctionTypeName [0] == 'F')) {
         fprintf (FvMapFile, "  0x%010llx    ", (unsigned long long) (ImageBaseAddress + FunctionAddress - LinkTimeBaseAddress));
-- 
1.9.5.msysgit.0



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

* [PATCH 18/52] BaseTools/GenFw: Fix parameter format mismatch in scanf functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (16 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 17/52] BaseTools/GenFv: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 19/52] BaseTools/GenVtf: " Hao Wu
                   ` (34 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenFw/GenFw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index c0d1e6a..c74d083 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -3198,7 +3198,7 @@ Returns:
     for (; *cptr && isspace((int)*cptr); cptr++) {
     }
     if (isxdigit ((int)*cptr)) {
-      if (sscanf (cptr, "%X", &ScannedData) != 1) {
+      if (sscanf (cptr, "%X", (INT32 *) &ScannedData) != 1) {
         return STATUS_ERROR;
       }
     }
-- 
1.9.5.msysgit.0



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

* [PATCH 19/52] BaseTools/GenVtf: Fix parameter format mismatch in scanf functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (17 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 18/52] BaseTools/GenFw: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 20/52] BaseTools/C/Common: Fix potential access over array bounds Hao Wu
                   ` (33 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenVtf/GenVtf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenVtf/GenVtf.c b/BaseTools/Source/C/GenVtf/GenVtf.c
index b68d86a..8e75d04 100644
--- a/BaseTools/Source/C/GenVtf/GenVtf.c
+++ b/BaseTools/Source/C/GenVtf/GenVtf.c
@@ -123,8 +123,8 @@ Returns:
     sscanf (
       Str,
       "%02x.%02x",
-      &Major,
-      &Minor
+      (INT32 *) &Major,
+      (INT32 *) &Minor
       );
   } else {
     Length = strlen(Str);
@@ -137,8 +137,8 @@ Returns:
     sscanf (
       TemStr,
       "%02x%02x",
-      &Major,
-      &Minor
+      (INT32 *) &Major,
+      (INT32 *) &Minor
       );
   }
 
-- 
1.9.5.msysgit.0



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

* [PATCH 20/52] BaseTools/C/Common: Fix potential access over array bounds
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (18 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 19/52] BaseTools/GenVtf: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 21/52] BaseTools/EfiRom: " Hao Wu
                   ` (32 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/Common/CommonLib.c  | 8 ++++++--
 BaseTools/Source/C/Common/Decompress.c | 7 +++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
index 2d07dfc..2f0aecf 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -1,7 +1,7 @@
 /** @file
 Common basic Library Functions
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -652,7 +652,11 @@ Returns:
     //
     // Construct the full file path
     //
-    strcat (mCommonLibFullPath, FileName);
+    if (strlen (mCommonLibFullPath) + strlen (FileName) > MAX_LONG_FILE_PATH - 1) {
+      Error (NULL, 0, 2000, "Invalid parameter", "FileName %s is too long!", FileName);
+      return NULL;
+    }
+    strncat (mCommonLibFullPath, FileName, MAX_LONG_FILE_PATH - strlen (mCommonLibFullPath) - 1);
     
     //
     // Convert directory separator '/' to '\\'
diff --git a/BaseTools/Source/C/Common/Decompress.c b/BaseTools/Source/C/Common/Decompress.c
index 48578ea..5768c86 100644
--- a/BaseTools/Source/C/Common/Decompress.c
+++ b/BaseTools/Source/C/Common/Decompress.c
@@ -2,7 +2,7 @@
 Decompressor. Algorithm Ported from OPSD code (Decomp.asm) for Efi and Tiano 
 compress algorithm.
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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
@@ -15,6 +15,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #include <stdlib.h>
 #include <string.h>
+#include <assert.h>
 #include "Decompress.h"
 
 //
@@ -240,7 +241,7 @@ Returns:
   for (Char = 0; Char < NumOfChar; Char++) {
 
     Len = BitLen[Char];
-    if (Len == 0) {
+    if (Len == 0 || Len >= 17) {
       continue;
     }
 
@@ -373,6 +374,8 @@ Returns:
   UINT16  Index;
   UINT32  Mask;
 
+  assert (nn <= NPT);
+
   Number = (UINT16) GetBits (Sd, nbit);
 
   if (Number == 0) {
-- 
1.9.5.msysgit.0



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

* [PATCH 21/52] BaseTools/EfiRom: Fix potential access over array bounds
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (19 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 20/52] BaseTools/C/Common: Fix potential access over array bounds Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 22/52] BaseTools/GenFv: " Hao Wu
                   ` (31 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/EfiRom/EfiRom.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/EfiRom/EfiRom.c b/BaseTools/Source/C/EfiRom/EfiRom.c
index 32aa3dc..b567c3b 100644
--- a/BaseTools/Source/C/EfiRom/EfiRom.c
+++ b/BaseTools/Source/C/EfiRom/EfiRom.c
@@ -979,7 +979,12 @@ Returns:
           Error (NULL, 0, 2000, "Invalid parameter", "Missing output file name with %s option!", Argv[0]);
           return STATUS_ERROR;
         }
-        strcpy (Options->OutFileName, Argv[1]);
+        if (strlen (Argv[1]) > MAX_PATH - 1) {
+          Error (NULL, 0, 2000, "Invalid parameter", "Output file name %s is too long!", Argv[1]);
+          return STATUS_ERROR;
+        }
+        strncpy (Options->OutFileName, Argv[1], MAX_PATH - 1);
+        Options->OutFileName[MAX_PATH - 1] = 0;
 
         Argv++;
         Argc--;
-- 
1.9.5.msysgit.0



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

* [PATCH 22/52] BaseTools/GenFv: Fix potential access over array bounds
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (20 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 21/52] BaseTools/EfiRom: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 23/52] BaseTools/TianoCompress: " Hao Wu
                   ` (30 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenFv/GenFv.c            |  9 +++--
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 55 +++++++++++++++++++++++++++--
 2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFv.c b/BaseTools/Source/C/GenFv/GenFv.c
index 01ae37a..4de24b9 100644
--- a/BaseTools/Source/C/GenFv/GenFv.c
+++ b/BaseTools/Source/C/GenFv/GenFv.c
@@ -4,7 +4,7 @@
   can be found in the Tiano Firmware Volume Generation Utility 
   Specification, review draft.
 
-Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2007 - 2016, 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        
@@ -337,7 +337,12 @@ Returns:
         Error (NULL, 0, 1003, "Invalid option value", "Input Ffsfile can't be null");
         return STATUS_ERROR;
       }
-      strcpy (mFvDataInfo.FvFiles[Index], argv[1]);
+      if (strlen (argv[1]) > MAX_LONG_FILE_PATH - 1) {
+        Error (NULL, 0, 1003, "Invalid option value", "Input Ffsfile name %s is too long!", argv[1]);
+        return STATUS_ERROR;
+      }
+      strncpy (mFvDataInfo.FvFiles[Index], argv[1], MAX_LONG_FILE_PATH - 1);
+      mFvDataInfo.FvFiles[Index][MAX_LONG_FILE_PATH - 1] = 0;
       DebugMsg (NULL, 0, 9, "FV component file", "the %uth name is %s", (unsigned) Index + 1, argv[1]);
       argc -= 2;
       argv += 2;
diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 8c769b4..d7c650e 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -374,7 +374,7 @@ Returns:
     }
   }
 
-  for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV; Index++) {
+  for (Index = 0; Number + Index < MAX_NUMBER_OF_FILES_IN_FV; Index++) {
     //
     // Read the FFS file list
     //
@@ -2418,17 +2418,19 @@ Returns:
   UINT8                           *FvImage;
   UINTN                           FvImageSize;
   FILE                            *FvFile;
-  CHAR8                           FvMapName [MAX_LONG_FILE_PATH];
+  CHAR8                           *FvMapName;
   FILE                            *FvMapFile;
   EFI_FIRMWARE_VOLUME_EXT_HEADER  *FvExtHeader;
   FILE                            *FvExtHeaderFile;
   UINTN                           FileSize;
-  CHAR8                           FvReportName[MAX_LONG_FILE_PATH];
+  CHAR8                           *FvReportName;
   FILE                            *FvReportFile;
 
   FvBufferHeader = NULL;
   FvFile         = NULL;
+  FvMapName      = NULL;
   FvMapFile      = NULL;
+  FvReportName   = NULL;
   FvReportFile   = NULL;
 
   if (InfFileImage != NULL) {
@@ -2566,8 +2568,34 @@ Returns:
   // FvMap file to log the function address of all modules in one Fvimage
   //
   if (MapFileName != NULL) {
+    if (strlen (MapFileName) > MAX_LONG_FILE_PATH - 1) {
+      Error (NULL, 0, 1003, "Invalid option value", "MapFileName %s is too long!", MapFileName);
+      Status = EFI_ABORTED;
+      goto Finish;
+    }
+
+    FvMapName = malloc (strlen (MapFileName) + 1);
+    if (FvMapName == NULL) {
+      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+      Status = EFI_OUT_OF_RESOURCES;
+      goto Finish;
+    }
+
     strcpy (FvMapName, MapFileName);
   } else {
+    if (strlen (FvFileName) + strlen (".map") > MAX_LONG_FILE_PATH - 1) {
+      Error (NULL, 0, 1003, "Invalid option value", "FvFileName %s is too long!", FvFileName);
+      Status = EFI_ABORTED;
+      goto Finish;
+    }
+
+    FvMapName = malloc (strlen (FvFileName) + strlen (".map") + 1);
+    if (FvMapName == NULL) {
+      Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+      Status = EFI_OUT_OF_RESOURCES;
+      goto Finish;
+    }
+
     strcpy (FvMapName, FvFileName);
     strcat (FvMapName, ".map");
   }
@@ -2576,6 +2604,19 @@ Returns:
   //
   // FvReport file to log the FV information in one Fvimage
   //
+  if (strlen (FvFileName) + strlen (".txt") > MAX_LONG_FILE_PATH - 1) {
+    Error (NULL, 0, 1003, "Invalid option value", "FvFileName %s is too long!", FvFileName);
+    Status = EFI_ABORTED;
+    goto Finish;
+  }
+
+  FvReportName = malloc (strlen (FvFileName) + strlen (".txt") + 1);
+  if (FvReportName == NULL) {
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Finish;
+  }
+
   strcpy (FvReportName, FvFileName);
   strcat (FvReportName, ".txt");
 
@@ -2852,6 +2893,14 @@ Finish:
   if (FvExtHeader != NULL) {
     free (FvExtHeader);
   }
+
+  if (FvMapName != NULL) {
+    free (FvMapName);
+  }
+
+  if (FvReportName != NULL) {
+    free (FvReportName);
+  }
   
   if (FvFile != NULL) {
     fflush (FvFile);
-- 
1.9.5.msysgit.0



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

* [PATCH 23/52] BaseTools/TianoCompress: Fix potential access over array bounds
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (21 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 22/52] BaseTools/GenFv: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 24/52] BaseTools/VfrCompile: " Hao Wu
                   ` (29 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index b994d93..93cb6c3 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -2215,7 +2215,7 @@ Returns:
   for (Char = 0; Char < NumOfChar; Char++) {
 
     Len = BitLen[Char];
-    if (Len == 0) {
+    if (Len == 0 || Len >= 17) {
       continue;
     }
 
@@ -2346,6 +2346,8 @@ Returns:
   volatile UINT16  Index;
   UINT32  Mask;
 
+  assert (nn <= NPT);
+
   Number = (UINT16) GetBits (Sd, nbit);
 
   if (Number == 0) {
-- 
1.9.5.msysgit.0



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

* [PATCH 24/52] BaseTools/VfrCompile: Fix potential access over array bounds
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (22 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 23/52] BaseTools/TianoCompress: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 25/52] BaseTools/VfrCompile: Avoid freeing memory with mismatched functions Hao Wu
                   ` (28 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h  |   3 +
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp   | 132 +++++++++++++++++++++---
 BaseTools/Source/C/VfrCompile/VfrCompiler.h     |   8 +-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp |   8 ++
 4 files changed, 135 insertions(+), 16 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h b/BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h
index 37cac24..f15bff1 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h
+++ b/BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h
@@ -30,6 +30,8 @@
  * 1989-2000
  */
 
+#include <assert.h>
+
 #define ZZINC {if ( track_columns ) (++_endcol);}
 
 #define ZZGETC {ch = input->nextChar(); cl = ZZSHIFT(ch);}
@@ -114,6 +116,7 @@ more:
 		state = dfa_base[automaton];
 		while (ZZNEWSTATE != DfaStates) {
 			state = newstate;
+            assert(state <= sizeof(dfa)/sizeof(dfa[0]));
 			ZZCOPY;
 			ZZGETC;
 			ZZINC;
diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index 59f4bf3..42c9a5e 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -69,13 +69,13 @@ CVfrCompiler::OptionInitialization (
   SetUtilityName ((CHAR8*) PROGRAM_NAME);
 
   mOptions.VfrFileName[0]                = '\0';
-  mOptions.RecordListFile[0]             = '\0';
+  mOptions.RecordListFile                = NULL;
   mOptions.CreateRecordListFile          = FALSE;
   mOptions.CreateIfrPkgFile              = FALSE;
-  mOptions.PkgOutputFileName[0]          = '\0';
-  mOptions.COutputFileName[0]            = '\0';
+  mOptions.PkgOutputFileName             = NULL;
+  mOptions.COutputFileName               = NULL;
   mOptions.OutputDirectory[0]            = '\0';
-  mOptions.PreprocessorOutputFileName[0] = '\0';
+  mOptions.PreprocessorOutputFileName    = NULL;
   mOptions.VfrBaseFileName[0]            = '\0';
   mOptions.IncludePaths                  = NULL;
   mOptions.SkipCPreprocessor             = TRUE;
@@ -119,7 +119,12 @@ CVfrCompiler::OptionInitialization (
         DebugError (NULL, 0, 1001, "Missing option", "-o missing output directory name");
         goto Fail;
       }
-      strcpy (mOptions.OutputDirectory, Argv[Index]);
+      if (strlen (Argv[Index]) > MAX_PATH - 1) {
+        DebugError (NULL, 0, 1003, "Invalid option value", "Output directory name %s is too long", Argv[Index]);
+        goto Fail;
+      }
+      strncpy (mOptions.OutputDirectory, Argv[Index], MAX_PATH - 1);
+      mOptions.OutputDirectory[MAX_PATH - 1] = 0;
       
       CHAR8 lastChar = mOptions.OutputDirectory[strlen(mOptions.OutputDirectory) - 1];
       if ((lastChar != '/') && (lastChar != '\\')) {
@@ -176,7 +181,12 @@ CVfrCompiler::OptionInitialization (
     DebugError (NULL, 0, 1001, "Missing option", "VFR file name is not specified.");
     goto Fail;
   } else {
-    strcpy (mOptions.VfrFileName, Argv[Index]);
+    if (strlen (Argv[Index]) > MAX_PATH) {
+      DebugError (NULL, 0, 1003, "Invalid option value", "VFR file name %s is too long.", Argv[Index]);
+      goto Fail;
+    }
+    strncpy (mOptions.VfrFileName, Argv[Index], MAX_PATH - 1);
+    mOptions.VfrFileName[MAX_PATH - 1] = 0;
   }
 
   if (SetBaseFileName() != 0) {
@@ -200,14 +210,26 @@ Fail:
   SET_RUN_STATUS (STATUS_DEAD);
 
   mOptions.VfrFileName[0]                = '\0';
-  mOptions.RecordListFile[0]             = '\0';
   mOptions.CreateRecordListFile          = FALSE;
   mOptions.CreateIfrPkgFile              = FALSE;
-  mOptions.PkgOutputFileName[0]          = '\0';
-  mOptions.COutputFileName[0]            = '\0';
   mOptions.OutputDirectory[0]            = '\0';
-  mOptions.PreprocessorOutputFileName[0] = '\0';
   mOptions.VfrBaseFileName[0]            = '\0';
+  if (mOptions.PkgOutputFileName != NULL) {
+    free (mOptions.PkgOutputFileName);
+    mOptions.PkgOutputFileName           = NULL;
+  }
+  if (mOptions.COutputFileName != NULL) {
+    free (mOptions.COutputFileName);
+    mOptions.COutputFileName             = NULL;
+  }
+  if (mOptions.PreprocessorOutputFileName != NULL) {
+    free (mOptions.PreprocessorOutputFileName);
+    mOptions.PreprocessorOutputFileName  = NULL;
+  }
+  if (mOptions.RecordListFile != NULL) {
+    free (mOptions.RecordListFile);
+    mOptions.RecordListFile              = NULL;
+  }
   if (mOptions.IncludePaths != NULL) {
     delete mOptions.IncludePaths;
     mOptions.IncludePaths                = NULL;
@@ -304,8 +326,14 @@ CVfrCompiler::SetBaseFileName (
     return -1;
   }
 
-  strncpy (mOptions.VfrBaseFileName, pFileName, pExt - pFileName);
-  mOptions.VfrBaseFileName[pExt - pFileName] = '\0';
+  *pExt = '\0';
+  if (strlen (pFileName) > MAX_PATH - 1) {
+    *pExt = '.';
+    return -1;
+  }
+  strncpy (mOptions.VfrBaseFileName, pFileName, MAX_PATH - 1);
+  mOptions.VfrBaseFileName[MAX_PATH - 1] = '\0';
+  *pExt = '.';
 
   return 0;
 }
@@ -315,10 +343,25 @@ CVfrCompiler::SetPkgOutputFileName (
   VOID
   )
 {
+  INTN Length;
+
   if (mOptions.VfrBaseFileName[0] == '\0') {
     return -1;
   }
 
+  Length = strlen (mOptions.OutputDirectory) +
+           strlen (mOptions.VfrBaseFileName) +
+           strlen (VFR_PACKAGE_FILENAME_EXTENSION) +
+           1;
+  if (Length > MAX_PATH) {
+    return -1;
+  }
+
+  mOptions.PkgOutputFileName = (CHAR8 *) malloc (Length);
+  if (mOptions.PkgOutputFileName == NULL) {
+    return -1;
+  }
+
   strcpy (mOptions.PkgOutputFileName, mOptions.OutputDirectory);
   strcat (mOptions.PkgOutputFileName, mOptions.VfrBaseFileName);
   strcat (mOptions.PkgOutputFileName, VFR_PACKAGE_FILENAME_EXTENSION);
@@ -331,10 +374,25 @@ CVfrCompiler::SetCOutputFileName (
   VOID
   )
 {
+  INTN Length;
+
   if (mOptions.VfrBaseFileName[0] == '\0') {
     return -1;
   }
 
+  Length = strlen (mOptions.OutputDirectory) +
+           strlen (mOptions.VfrBaseFileName) +
+           strlen (".c") +
+           1;
+  if (Length > MAX_PATH) {
+    return -1;
+  }
+
+  mOptions.COutputFileName = (CHAR8 *) malloc (Length);
+  if (mOptions.COutputFileName == NULL) {
+    return -1;
+  }
+
   strcpy (mOptions.COutputFileName, mOptions.OutputDirectory);
   strcat (mOptions.COutputFileName, mOptions.VfrBaseFileName);
   strcat (mOptions.COutputFileName, ".c");
@@ -347,10 +405,25 @@ CVfrCompiler::SetPreprocessorOutputFileName (
   VOID
   )
 {
+  INTN Length;
+
   if (mOptions.VfrBaseFileName[0] == '\0') {
     return -1;
   }
 
+  Length = strlen (mOptions.OutputDirectory) +
+           strlen (mOptions.VfrBaseFileName) +
+           strlen (VFR_PREPROCESS_FILENAME_EXTENSION) +
+           1;
+  if (Length > MAX_PATH) {
+    return -1;
+  }
+
+  mOptions.PreprocessorOutputFileName = (CHAR8 *) malloc (Length);
+  if (mOptions.PreprocessorOutputFileName == NULL) {
+    return -1;
+  }
+
   strcpy (mOptions.PreprocessorOutputFileName, mOptions.OutputDirectory);
   strcat (mOptions.PreprocessorOutputFileName, mOptions.VfrBaseFileName);
   strcat (mOptions.PreprocessorOutputFileName, VFR_PREPROCESS_FILENAME_EXTENSION);
@@ -363,10 +436,25 @@ CVfrCompiler::SetRecordListFileName (
   VOID
   )
 {
+  INTN Length;
+
   if (mOptions.VfrBaseFileName[0] == '\0') {
     return -1;
   }
 
+  Length = strlen (mOptions.OutputDirectory) +
+           strlen (mOptions.VfrBaseFileName) +
+           strlen (VFR_RECORDLIST_FILENAME_EXTENSION) +
+           1;
+  if (Length > MAX_PATH) {
+    return -1;
+  }
+
+  mOptions.RecordListFile = (CHAR8 *) malloc (Length);
+  if (mOptions.RecordListFile == NULL) {
+    return -1;
+  }
+
   strcpy (mOptions.RecordListFile, mOptions.OutputDirectory);
   strcat (mOptions.RecordListFile, mOptions.VfrBaseFileName);
   strcat (mOptions.RecordListFile, VFR_RECORDLIST_FILENAME_EXTENSION);
@@ -397,6 +485,26 @@ CVfrCompiler::~CVfrCompiler (
   VOID
   )
 {
+  if (mOptions.PkgOutputFileName != NULL) {
+    free (mOptions.PkgOutputFileName);
+    mOptions.PkgOutputFileName = NULL;
+  }
+
+  if (mOptions.COutputFileName != NULL) {
+    free (mOptions.COutputFileName);
+    mOptions.COutputFileName = NULL;
+  }
+
+  if (mOptions.PreprocessorOutputFileName != NULL) {
+    free (mOptions.PreprocessorOutputFileName);
+    mOptions.PreprocessorOutputFileName = NULL;
+  }
+
+  if (mOptions.RecordListFile != NULL) {
+    free (mOptions.RecordListFile);
+    mOptions.RecordListFile = NULL;
+  }
+
   if (mOptions.IncludePaths != NULL) {
     delete mOptions.IncludePaths;
     mOptions.IncludePaths = NULL;
diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.h b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
index 7dd9dd0..7200cea 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.h
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
@@ -42,13 +42,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 typedef struct {
   CHAR8   VfrFileName[MAX_PATH];
-  CHAR8   RecordListFile[MAX_PATH];
-  CHAR8   PkgOutputFileName[MAX_PATH];
-  CHAR8   COutputFileName[MAX_PATH];
+  CHAR8   *RecordListFile;
+  CHAR8   *PkgOutputFileName;
+  CHAR8   *COutputFileName;
   bool    CreateRecordListFile;
   bool    CreateIfrPkgFile;
   CHAR8   OutputDirectory[MAX_PATH];
-  CHAR8   PreprocessorOutputFileName[MAX_PATH];
+  CHAR8   *PreprocessorOutputFileName;
   CHAR8   VfrBaseFileName[MAX_PATH];  // name of input VFR file with no path or extension
   CHAR8   *IncludePaths;
   bool    SkipCPreprocessor;
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index 1ab95be..24b0bfa 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -1474,6 +1474,10 @@ CVfrDataStorage::GetFreeVarStoreId (
     }
   }
 
+  if (Index == EFI_FREE_VARSTORE_ID_BITMAP_SIZE) {
+    return EFI_VARSTORE_ID_INVALID;
+  }
+
   for (Offset = 0, Mask = 0x80000000; Mask != 0; Mask >>= 1, Offset++) {
     if ((mFreeVarStoreIdBitMap[Index] & Mask) == 0) {
       mFreeVarStoreIdBitMap[Index] |= Mask;
@@ -2437,6 +2441,10 @@ CVfrQuestionDB::GetFreeQuestionId (
     }
   }
 
+  if (Index == EFI_FREE_QUESTION_ID_BITMAP_SIZE) {
+    return EFI_QUESTION_ID_INVALID;
+  }
+
   for (Offset = 0, Mask = 0x80000000; Mask != 0; Mask >>= 1, Offset++) {
     if ((mFreeQIdBitMap[Index] & Mask) == 0) {
       mFreeQIdBitMap[Index] |= Mask;
-- 
1.9.5.msysgit.0



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

* [PATCH 25/52] BaseTools/VfrCompile: Avoid freeing memory with mismatched functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (23 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 24/52] BaseTools/VfrCompile: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 26/52] BaseTools/VfrCompile: Add assignment operator definition for some classes Hao Wu
                   ` (27 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

Memory allocated by operator new[] should be freed using delete[] to avoid
possible memory leak.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp   |  4 ++--
 BaseTools/Source/C/VfrCompile/VfrError.cpp      |  4 ++--
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp    |  6 +++---
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 24 ++++++++++++------------
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  4 +++-
 5 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index 42c9a5e..0e5c7c6 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -628,7 +628,7 @@ CVfrCompiler::PreProcess (
     goto Fail;
   }
 
-  delete PreProcessCmd;
+  delete[] PreProcessCmd;
 
 Out:
   SET_RUN_STATUS (STATUS_PREPROCESSED);
@@ -638,7 +638,7 @@ Fail:
   if (!IS_RUN_STATUS(STATUS_DEAD)) {
     SET_RUN_STATUS (STATUS_FAILED);
   }
-  delete PreProcessCmd;
+  delete[] PreProcessCmd;
 }
 
 extern UINT8 VfrParserStart (IN FILE *, IN INPUT_INFO_TO_SYNTAX *);
diff --git a/BaseTools/Source/C/VfrCompile/VfrError.cpp b/BaseTools/Source/C/VfrCompile/VfrError.cpp
index 3c506ec..285e175 100644
--- a/BaseTools/Source/C/VfrCompile/VfrError.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrError.cpp
@@ -2,7 +2,7 @@
   
   VfrCompiler error handler.
 
-Copyright (c) 2004 - 2013, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -145,7 +145,7 @@ SVfrFileScopeRecord::~SVfrFileScopeRecord (
   )
 {
   if (mFileName != NULL) {
-    delete mFileName;
+    delete[] mFileName;
   }
 }
 
diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
index 124b8e8..9c76b29 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
@@ -56,13 +56,13 @@ SPendingAssign::~SPendingAssign (
   )
 {
   if (mKey != NULL) {
-    delete mKey;
+    delete[] mKey;
   }
   mAddr   = NULL;
   mLen    = 0;
   mLineNo = 0;
   if (mMsg != NULL) {
-    delete mMsg;
+    delete[] mMsg;
   }
   mNext   = NULL;
 }
@@ -898,7 +898,7 @@ CFormPkg::DeclarePendingQuestion (
             strcpy (NewStr, SName);
             strcat (NewStr, VarStr + strlen (FName));
             ReturnCode = lCVfrVarDataTypeDB.GetDataFieldInfo (NewStr, Info.mInfo.mVarOffset, Info.mVarType, Info.mVarTotalSize);
-            delete NewStr;
+            delete[] NewStr;
           }
         } else {
           ReturnCode = VFR_RETURN_UNSUPPORTED;
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index 24b0bfa..1afa5a2 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -123,7 +123,7 @@ SConfigInfo::~SConfigInfo (
   VOID
   )
 {
-  BUFFER_SAFE_FREE (mValue);
+  ARRAY_SAFE_FREE (mValue);
 }
 
 SConfigItem::SConfigItem (
@@ -200,9 +200,9 @@ SConfigItem::~SConfigItem (
 {
   SConfigInfo  *Info;
 
-  BUFFER_SAFE_FREE (mName);
-  BUFFER_SAFE_FREE (mGuid);
-  BUFFER_SAFE_FREE (mId);
+  ARRAY_SAFE_FREE (mName);
+  ARRAY_SAFE_FREE (mGuid);
+  ARRAY_SAFE_FREE (mId);
   while (mInfoStrList != NULL) {
     Info = mInfoStrList;
     mInfoStrList = mInfoStrList->mNext;
@@ -1393,7 +1393,7 @@ SVfrVarStorageNode::~SVfrVarStorageNode (
   )
 {
   if (mVarStoreName != NULL) {
-    delete mVarStoreName;
+    delete[] mVarStoreName;
   }
 
   if (mVarStoreType == EFI_VFR_VARSTORE_NAME) {
@@ -2102,7 +2102,7 @@ SVfrDefaultStoreNode::~SVfrDefaultStoreNode (
   )
 {
   if (mRefName != NULL) {
-    delete mRefName;
+    delete[] mRefName;
   }
 }
 
@@ -2304,7 +2304,7 @@ SVfrRuleNode::~SVfrRuleNode (
   )
 {
   if (mRuleName != NULL) {
-    delete mRuleName;
+    delete[] mRuleName;
   }
 }
 
@@ -2523,11 +2523,11 @@ SVfrQuestionNode::~SVfrQuestionNode (
   )
 {
   if (mName != NULL) {
-    delete mName;
+    delete[] mName;
   }
 
   if (mVarIdStr != NULL) {
-    delete mVarIdStr;
+    delete[] mVarIdStr;
   }
 }
 
@@ -3387,7 +3387,7 @@ CVfrStringDB::GetVarStoreNameFormStringId (
   // Check the String package.
   //
   if (PkgHeader->Header.Type != EFI_HII_PACKAGE_STRINGS) {
-    delete StringPtr;
+    delete[] StringPtr;
     return NULL;
   }
 
@@ -3414,7 +3414,7 @@ CVfrStringDB::GetVarStoreNameFormStringId (
   //
   Status = FindStringBlock(Current, StringId, &NameOffset, &BlockType);
   if (Status != EFI_SUCCESS) {
-    delete StringPtr;
+    delete[] StringPtr;
     return NULL;
   }
 
@@ -3447,7 +3447,7 @@ CVfrStringDB::GetVarStoreNameFormStringId (
     break;
   }
 
-  delete StringPtr;
+  delete[] StringPtr;
 
   return VarStoreName;
 }
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
index 5faa1f4..35d17a0 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
@@ -33,6 +33,8 @@ extern BOOLEAN  VfrCompatibleMode;
 #define EFI_BITS_PER_UINT32                (1 << EFI_BITS_SHIFT_PER_UINT32)
 
 #define BUFFER_SAFE_FREE(Buf)              do { if ((Buf) != NULL) { delete (Buf); } } while (0);
+#define ARRAY_SAFE_FREE(Buf)               do { if ((Buf) != NULL) { delete[] (Buf); } } while (0);
+
 
 class CVfrBinaryOutput {
 public:
@@ -139,7 +141,7 @@ struct SVfrPackStackNode {
 
   ~SVfrPackStackNode (VOID) {
     if (mIdentifier != NULL) {
-      delete mIdentifier;
+      delete[] mIdentifier;
     }
     mNext = NULL;
   }
-- 
1.9.5.msysgit.0



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

* [PATCH 26/52] BaseTools/VfrCompile: Add assignment operator definition for some classes
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (24 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 25/52] BaseTools/VfrCompile: Avoid freeing memory with mismatched functions Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 27/52] BaseTools/VfrCompile: Avoid freeing freed memory in classes Hao Wu
                   ` (26 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

For class that defines the copy constructor, it is better to add the
assignment operator definition as well.

This commit adds the definition for assignment operator for the classes
with the copy constructor defined.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrFormPkg.h      |  1 +
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 16 ++++++++++++++++
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  1 +
 3 files changed, 18 insertions(+)

diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
index 051df28..3c7964a 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
@@ -307,6 +307,7 @@ private:
 public:
   CIfrOpHeader (IN UINT8 OpCode, IN VOID *StartAddr, IN UINT8 Length = 0);
   CIfrOpHeader (IN CIfrOpHeader &);
+  CIfrOpHeader& operator=(IN CONST CIfrOpHeader &);
 
   VOID IncLength (UINT8 Size) {
     if ((mHeader->Length + Size) > mHeader->Length) {
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index 1afa5a2..3ca57ed 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -2390,6 +2390,22 @@ EFI_VARSTORE_INFO::EFI_VARSTORE_INFO (
   mVarTotalSize    = Info.mVarTotalSize;
 }
 
+EFI_VARSTORE_INFO&
+EFI_VARSTORE_INFO::operator= (
+  IN CONST EFI_VARSTORE_INFO &Info
+  )
+{
+  if (this != &Info) {
+    mVarStoreId      = Info.mVarStoreId;
+    mInfo.mVarName   = Info.mInfo.mVarName;
+    mInfo.mVarOffset = Info.mInfo.mVarOffset;
+    mVarType         = Info.mVarType;
+    mVarTotalSize    = Info.mVarTotalSize;
+  }
+
+  return *this;
+}
+
 BOOLEAN
 EFI_VARSTORE_INFO::operator == (
   IN EFI_VARSTORE_INFO  *Info
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
index 35d17a0..2e06e4f 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
@@ -264,6 +264,7 @@ struct EFI_VARSTORE_INFO {
 
   EFI_VARSTORE_INFO (VOID);
   EFI_VARSTORE_INFO (IN EFI_VARSTORE_INFO &);
+  EFI_VARSTORE_INFO& operator=(IN CONST EFI_VARSTORE_INFO &);
   BOOLEAN operator == (IN EFI_VARSTORE_INFO *);
 };
 
-- 
1.9.5.msysgit.0



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

* [PATCH 27/52] BaseTools/VfrCompile: Avoid freeing freed memory in classes
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (25 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 26/52] BaseTools/VfrCompile: Add assignment operator definition for some classes Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 28/52] BaseTools/VfrCompile: Remove unused local variables Hao Wu
                   ` (25 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

For classes that contain dynamically allocated data members, copy
constructor and assignment operator should be implemented or both
operations should be prohibited to avoid freeing freed memory caused by
shallow copy.

This commit declares both copy constructor and assignment operator as
'private' for classes that contain dynamically allocated data members.
This will prevent freeing already freed memory.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h |  3 ++
 BaseTools/Source/C/VfrCompile/VfrError.h           | 10 +++-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.h         | 12 +++++
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h      | 55 ++++++++++++++++++++++
 4 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h b/BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h
index db6cc18..667ecfd 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h
+++ b/BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h
@@ -119,6 +119,9 @@ public:
 
 /* user must subclass this */
 class DllExportPCCTS DLGLexerBase : public ANTLRTokenStream {
+private:
+    DLGLexerBase(const DLGLexerBase&);             // Prevent copy-construction
+    DLGLexerBase& operator=(const DLGLexerBase&);  // Prevent assignment
 public:
 	virtual ANTLRTokenType erraction();
 
diff --git a/BaseTools/Source/C/VfrCompile/VfrError.h b/BaseTools/Source/C/VfrCompile/VfrError.h
index 8241ce2..4dbc54c 100644
--- a/BaseTools/Source/C/VfrCompile/VfrError.h
+++ b/BaseTools/Source/C/VfrCompile/VfrError.h
@@ -2,7 +2,7 @@
   
   VfrCompiler Error definition
 
-Copyright (c) 2004 - 2013, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -73,6 +73,10 @@ struct SVfrFileScopeRecord {
 
   SVfrFileScopeRecord (IN CHAR8 *, IN UINT32);
   ~SVfrFileScopeRecord();
+
+private:
+  SVfrFileScopeRecord (IN CONST SVfrFileScopeRecord&);             // Prevent copy-construction
+  SVfrFileScopeRecord& operator= (IN CONST SVfrFileScopeRecord&);  // Prevent assignment
 };
 
 class CVfrErrorHandle {
@@ -95,6 +99,10 @@ public:
   UINT8 HandleError (IN EFI_VFR_RETURN_CODE, IN UINT32 LineNum = 0, IN CHAR8 *TokName = NULL);
   UINT8 HandleWarning (IN EFI_VFR_WARNING_CODE, IN UINT32 LineNum = 0, IN CHAR8 *TokName = NULL);
   VOID  PrintMsg (IN UINT32 LineNum = 0, IN CHAR8 *TokName = NULL, IN CONST CHAR8 *MsgType = "Error", IN CONST CHAR8 *ErrorMsg = "");
+
+private:
+  CVfrErrorHandle (IN CONST CVfrErrorHandle&);             // Prevent copy-construction
+  CVfrErrorHandle& operator= (IN CONST CVfrErrorHandle&);  // Prevent assignment
 };
 
 #define CHECK_ERROR_RETURN(f, v) do { EFI_VFR_RETURN_CODE r; if ((r = (f)) != (v)) { return r; } } while (0)
diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
index 3c7964a..17ab14c 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
@@ -87,6 +87,10 @@ struct SPendingAssign {
   VOID   SetAddrAndLen (IN VOID *, IN UINT32);
   VOID   AssignValue (IN VOID *, IN UINT32);
   CHAR8 * GetKey (VOID);
+
+private:
+  SPendingAssign (IN CONST SPendingAssign&);             // Prevent copy-construction
+  SPendingAssign& operator= (IN CONST SPendingAssign&);  // Prevent assignment
 };
 
 struct SBufferNode {
@@ -139,6 +143,10 @@ public:
   EFI_VFR_RETURN_CODE BuildPkg (OUT PACKAGE_DATA &);
   EFI_VFR_RETURN_CODE GenCFile (IN CHAR8 *, IN FILE *, IN PACKAGE_DATA *PkgData = NULL);
 
+private:
+  CFormPkg (IN CONST CFormPkg&);             // Prevent copy-construction
+  CFormPkg& operator= (IN CONST CFormPkg&);  // Prevent assignment
+
 public:
   EFI_VFR_RETURN_CODE AssignPending (IN CHAR8 *, IN VOID *, IN UINT32, IN UINT32, IN CONST CHAR8 *Msg = NULL);
   VOID                DoPendingAssign (IN CHAR8 *, IN VOID *, IN UINT32);
@@ -237,6 +245,10 @@ public:
   VOID        IfrCreateDefaultForQuestion (IN  SIfrRecord *, IN  QuestionDefaultRecord *);
   VOID        IfrParseDefaulInfoInQuestion (IN  SIfrRecord *, OUT QuestionDefaultRecord *);
   VOID        IfrAddDefaultToBufferConfig (IN  UINT16, IN  SIfrRecord *,IN  EFI_IFR_TYPE_VALUE);
+
+private:
+  CIfrRecordInfoDB (IN CONST CIfrRecordInfoDB&);             // Prevent copy-construction
+  CIfrRecordInfoDB& operator= (IN CONST CIfrRecordInfoDB&);  // Prevent assignment
 };
 
 extern CIfrRecordInfoDB gCIfrRecordInfoDB;
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
index 2e06e4f..59509c3 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
@@ -55,6 +55,10 @@ struct SConfigInfo {
 
   SConfigInfo (IN UINT8, IN UINT16, IN UINT32, IN EFI_IFR_TYPE_VALUE);
   ~SConfigInfo (VOID);
+
+private:
+  SConfigInfo (IN CONST SConfigInfo&);             // Prevent copy-construction
+  SConfigInfo& operator= (IN CONST SConfigInfo&);  // Prevent assignment
 };
 
 struct SConfigItem {
@@ -68,6 +72,10 @@ public:
   SConfigItem (IN CHAR8 *, IN EFI_GUID *, IN CHAR8 *);
   SConfigItem (IN CHAR8 *, IN EFI_GUID *, IN CHAR8 *, IN UINT8, IN UINT16, IN UINT16, IN EFI_IFR_TYPE_VALUE);
   virtual ~SConfigItem ();
+
+private:
+  SConfigItem (IN CONST SConfigItem&);             // Prevent copy-construction
+  SConfigItem& operator= (IN CONST SConfigItem&);  // Prevent assignment
 };
 
 class CVfrBufferConfig {
@@ -90,6 +98,10 @@ public:
 #endif
   virtual VOID    Close (VOID);
   virtual VOID    OutputCFile (IN FILE *, IN CHAR8 *);
+
+private:
+  CVfrBufferConfig (IN CONST CVfrBufferConfig&);             // Prevent copy-construction
+  CVfrBufferConfig& operator= (IN CONST CVfrBufferConfig&);  // Prevent assignment
 };
 
 extern CVfrBufferConfig gCVfrBufferConfig;
@@ -157,6 +169,10 @@ struct SVfrPackStackNode {
       return FALSE;
     }
   }
+
+private:
+  SVfrPackStackNode (IN CONST SVfrPackStackNode&);             // Prevent copy-construction
+  SVfrPackStackNode& operator= (IN CONST SVfrPackStackNode&);  // Prevent assignment
 };
 
 class CVfrVarDataTypeDB {
@@ -210,6 +226,10 @@ public:
 #ifdef CVFR_VARDATATYPEDB_DEBUG
   VOID ParserDB ();
 #endif
+
+private:
+  CVfrVarDataTypeDB (IN CONST CVfrVarDataTypeDB&);             // Prevent copy-construction
+  CVfrVarDataTypeDB& operator= (IN CONST CVfrVarDataTypeDB&);  // Prevent assignment
 };
 
 extern CVfrVarDataTypeDB  gCVfrVarDataTypeDB;
@@ -251,6 +271,10 @@ public:
   SVfrVarStorageNode (IN EFI_GUID *, IN CHAR8 *, IN EFI_VARSTORE_ID, IN SVfrDataType *, IN BOOLEAN Flag = TRUE);
   SVfrVarStorageNode (IN CHAR8 *, IN EFI_VARSTORE_ID);
   ~SVfrVarStorageNode (VOID);
+
+private:
+  SVfrVarStorageNode (IN CONST SVfrVarStorageNode&);             // Prevent copy-construction
+  SVfrVarStorageNode& operator= (IN CONST SVfrVarStorageNode&);  // Prevent assignment
 };
 
 struct EFI_VARSTORE_INFO {
@@ -332,6 +356,10 @@ public:
   EFI_VFR_RETURN_CODE GetNameVarStoreInfo (IN EFI_VARSTORE_INFO *, IN UINT32);
   EFI_VFR_RETURN_CODE AddBufferVarStoreFieldInfo (IN EFI_VARSTORE_INFO *);
   EFI_VFR_RETURN_CODE GetBufferVarStoreFieldInfo (IN OUT EFI_VARSTORE_INFO *);
+
+private:
+  CVfrDataStorage (IN CONST CVfrDataStorage&);             // Prevent copy-construction
+  CVfrDataStorage& operator= (IN CONST CVfrDataStorage&);  // Prevent assignment
 };
 
 extern CVfrDataStorage gCVfrDataStorage;
@@ -357,6 +385,10 @@ struct SVfrQuestionNode {
 
   SVfrQuestionNode (IN CHAR8 *, IN CHAR8 *, IN UINT32 BitMask = 0);
   ~SVfrQuestionNode ();
+
+private:
+  SVfrQuestionNode (IN CONST SVfrQuestionNode&);             // Prevent copy-construction
+  SVfrQuestionNode& operator= (IN CONST SVfrQuestionNode&);  // Prevent assignment
 };
 
 class CVfrQuestionDB {
@@ -390,6 +422,10 @@ public:
   VOID SetCompatibleMode (IN BOOLEAN Mode) {
     VfrCompatibleMode = Mode;
   }
+
+private:
+  CVfrQuestionDB (IN CONST CVfrQuestionDB&);             // Prevent copy-construction
+  CVfrQuestionDB& operator= (IN CONST CVfrQuestionDB&);  // Prevent assignment
 };
 
 struct SVfrDefaultStoreNode {
@@ -402,6 +438,10 @@ struct SVfrDefaultStoreNode {
 
   SVfrDefaultStoreNode (IN EFI_IFR_DEFAULTSTORE *, IN CHAR8 *, IN EFI_STRING_ID, IN UINT16);
   ~SVfrDefaultStoreNode();
+
+private:
+  SVfrDefaultStoreNode (IN CONST SVfrDefaultStoreNode&);             // Prevent copy-construction
+  SVfrDefaultStoreNode& operator= (IN CONST SVfrDefaultStoreNode&);  // Prevent assignment
 };
 
 class CVfrDefaultStore {
@@ -417,6 +457,10 @@ public:
   BOOLEAN             DefaultIdRegistered (IN UINT16);
   EFI_VFR_RETURN_CODE GetDefaultId (IN CHAR8 *, OUT UINT16 *);
   EFI_VFR_RETURN_CODE BufferVarStoreAltConfigAdd (IN EFI_VARSTORE_ID, IN EFI_VARSTORE_INFO &, IN CHAR8 *, IN EFI_GUID *, IN UINT8, IN EFI_IFR_TYPE_VALUE);
+
+private:
+  CVfrDefaultStore (IN CONST CVfrDefaultStore&);             // Prevent copy-construction
+  CVfrDefaultStore& operator= (IN CONST CVfrDefaultStore&);  // Prevent assignment
 };
 
 extern CVfrDefaultStore gCVfrDefaultStore;
@@ -431,6 +475,10 @@ struct SVfrRuleNode {
 
   SVfrRuleNode(IN CHAR8 *, IN UINT8);
   ~SVfrRuleNode();
+
+private:
+  SVfrRuleNode (IN CONST SVfrRuleNode&);             // Prevent copy-construction
+  SVfrRuleNode& operator= (IN CONST SVfrRuleNode&);  // Prevent assignment
 };
 
 class CVfrRulesDB {
@@ -444,6 +492,10 @@ public:
 
   VOID RegisterRule (IN CHAR8 *);
   UINT8 GetRuleId (IN CHAR8 *);
+
+private:
+  CVfrRulesDB (IN CONST CVfrRulesDB&);             // Prevent copy-construction
+  CVfrRulesDB& operator= (IN CONST CVfrRulesDB&);  // Prevent assignment
 };
 
 class CVfrStringDB {
@@ -478,6 +530,9 @@ public:
     IN EFI_STRING_ID StringId
     );
 
+private:
+  CVfrStringDB (IN CONST CVfrStringDB&);             // Prevent copy-construction
+  CVfrStringDB& operator= (IN CONST CVfrStringDB&);  // Prevent assignment
 };
 
 #endif
-- 
1.9.5.msysgit.0



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

* [PATCH 28/52] BaseTools/VfrCompile: Remove unused local variables
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (26 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 27/52] BaseTools/VfrCompile: Avoid freeing freed memory in classes Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 29/52] BaseTools/C/Common: Fix potential memory leak Hao Wu
                   ` (24 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
index 9c76b29..d06c1bc 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
@@ -828,7 +828,6 @@ CFormPkg::DeclarePendingQuestion (
   UINT32         ShrinkSize = 0;
   EFI_VFR_RETURN_CODE  ReturnCode;
   EFI_VFR_VARSTORE_TYPE VarStoreType  = EFI_VFR_VARSTORE_INVALID;
-  EFI_VARSTORE_ID       VarStoreId    = EFI_VARSTORE_ID_INVALID;
 
   //
   // Declare all questions as Numeric in DisableIf True
@@ -1410,7 +1409,6 @@ CIfrRecordInfoDB::IfrRecordAdjust (
   EFI_QUESTION_ID    QuestionId;
   UINT32             StackCount;
   UINT32             QuestionScope;
-  UINT32             OpcodeOffset;
   CHAR8              ErrorMsg[MAX_STRING_LEN] = {0, };
   EFI_VFR_RETURN_CODE  Status;
 
-- 
1.9.5.msysgit.0



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

* [PATCH 29/52] BaseTools/C/Common: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (27 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 28/52] BaseTools/VfrCompile: Remove unused local variables Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 30/52] BaseTools/EfiRom: " Hao Wu
                   ` (23 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/Common/Decompress.c             | 34 ++++++++++++++++------
 BaseTools/Source/C/Common/FirmwareVolumeBuffer.c   |  1 +
 BaseTools/Source/C/Common/MemoryFile.c             |  3 +-
 .../Source/C/Common/ParseGuidedSectionTools.c      |  6 ++++
 4 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/BaseTools/Source/C/Common/Decompress.c b/BaseTools/Source/C/Common/Decompress.c
index 5768c86..61c8199 100644
--- a/BaseTools/Source/C/Common/Decompress.c
+++ b/BaseTools/Source/C/Common/Decompress.c
@@ -934,7 +934,9 @@ Extract (
   UINT32        ScratchSize;
   EFI_STATUS    Status;
 
-  Status = EFI_SUCCESS;
+  Scratch = NULL;
+  Status  = EFI_SUCCESS;
+
   switch (Algorithm) {
   case 0:
     *Destination = (VOID *)malloc(SrcSize);
@@ -948,30 +950,44 @@ Extract (
     Status = EfiGetInfo(Source, SrcSize, DstSize, &ScratchSize);
     if (Status == EFI_SUCCESS) {
       Scratch = (VOID *)malloc(ScratchSize);
+      if (Scratch == NULL) {
+        return EFI_OUT_OF_RESOURCES;
+      }
+
       *Destination = (VOID *)malloc(*DstSize);
-      if (Scratch != NULL && *Destination != NULL) {
-        Status = EfiDecompress(Source, SrcSize, *Destination, *DstSize, Scratch, ScratchSize);
-      } else {
-        Status = EFI_OUT_OF_RESOURCES;
+      if (*Destination == NULL) {
+        free (Scratch);
+        return EFI_OUT_OF_RESOURCES;
       }
+
+      Status = EfiDecompress(Source, SrcSize, *Destination, *DstSize, Scratch, ScratchSize);
     }
     break;
   case 2:
     Status = TianoGetInfo(Source, SrcSize, DstSize, &ScratchSize);
     if (Status == EFI_SUCCESS) {
       Scratch = (VOID *)malloc(ScratchSize);
+      if (Scratch == NULL) {
+        return EFI_OUT_OF_RESOURCES;
+      }
+
       *Destination = (VOID *)malloc(*DstSize);
-      if (Scratch != NULL && *Destination != NULL) {
-        Status = TianoDecompress(Source, SrcSize, *Destination, *DstSize, Scratch, ScratchSize);
-      } else {
-        Status = EFI_OUT_OF_RESOURCES;
+      if (*Destination == NULL) {
+        free (Scratch);
+        return EFI_OUT_OF_RESOURCES;
       }
+
+      Status = TianoDecompress(Source, SrcSize, *Destination, *DstSize, Scratch, ScratchSize);
     }
     break;
   default:
     Status = EFI_INVALID_PARAMETER;
   }
 
+  if (Scratch != NULL) {
+    free (Scratch);
+  }
+
   return Status;
 }
 
diff --git a/BaseTools/Source/C/Common/FirmwareVolumeBuffer.c b/BaseTools/Source/C/Common/FirmwareVolumeBuffer.c
index a287fe1..d4a6353 100644
--- a/BaseTools/Source/C/Common/FirmwareVolumeBuffer.c
+++ b/BaseTools/Source/C/Common/FirmwareVolumeBuffer.c
@@ -186,6 +186,7 @@ Returns:
 
   Status = FvBufClearAllFiles (TempFv);
   if (EFI_ERROR (Status)) {
+    CommonLibBinderFree (TempFv);
     return Status;
   }
 
diff --git a/BaseTools/Source/C/Common/MemoryFile.c b/BaseTools/Source/C/Common/MemoryFile.c
index 00ea0c6..1d90688 100644
--- a/BaseTools/Source/C/Common/MemoryFile.c
+++ b/BaseTools/Source/C/Common/MemoryFile.c
@@ -1,7 +1,7 @@
 /** @file
 This contains some useful functions for accessing files.
 
-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2016, 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        
@@ -69,6 +69,7 @@ Returns:
 
   NewMemoryFile = malloc (sizeof (*NewMemoryFile));
   if (NewMemoryFile == NULL) {
+    free (InputFileImage);
     return EFI_OUT_OF_RESOURCES;
   }
 
diff --git a/BaseTools/Source/C/Common/ParseGuidedSectionTools.c b/BaseTools/Source/C/Common/ParseGuidedSectionTools.c
index fc8f488..115cfa4 100644
--- a/BaseTools/Source/C/Common/ParseGuidedSectionTools.c
+++ b/BaseTools/Source/C/Common/ParseGuidedSectionTools.c
@@ -125,10 +125,12 @@ Returns:
     
     Status = StripInfDscStringInPlace (NextLine);
     if (EFI_ERROR (Status)) {
+      free (NextLine);
       break;
     }
 
     if (NextLine[0] == '\0') {
+      free (NextLine);
       continue;
     }
 
@@ -153,8 +155,12 @@ Returns:
           LastGuidTool = NewGuidTool;
         }
       }
+    }
+
+    if (Tool != NULL) {
       FreeStringList (Tool);
     }
+    free (NextLine);
   }
 
   return FirstGuidTool;
-- 
1.9.5.msysgit.0



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

* [PATCH 30/52] BaseTools/EfiRom: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (28 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 29/52] BaseTools/C/Common: Fix potential memory leak Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 31/52] BaseTools/GenFv: " Hao Wu
                   ` (22 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/EfiRom/EfiRom.c | 70 +++++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 19 deletions(-)

diff --git a/BaseTools/Source/C/EfiRom/EfiRom.c b/BaseTools/Source/C/EfiRom/EfiRom.c
index b567c3b..e862989 100644
--- a/BaseTools/Source/C/EfiRom/EfiRom.c
+++ b/BaseTools/Source/C/EfiRom/EfiRom.c
@@ -883,9 +883,11 @@ Returns:
   UINT32    ClassCode;
   UINT32    CodeRevision;
   EFI_STATUS Status;
+  INTN       ReturnStatus;
   BOOLEAN    EfiRomFlag;
   UINT64     TempValue;
 
+  ReturnStatus = 0;
   FileFlags = 0;
   EfiRomFlag = FALSE;
 
@@ -940,11 +942,13 @@ Returns:
         Status = AsciiStringToUint64(Argv[1], FALSE, &TempValue);
         if (EFI_ERROR (Status)) {
           Error (NULL, 0, 2000, "Invalid option value", "%s = %s", Argv[0], Argv[1]);
-          return 1;
+          ReturnStatus = 1;
+          goto Done;
         }
         if (TempValue >= 0x10000) {
           Error (NULL, 0, 2000, "Invalid option value", "Vendor Id %s out of range!", Argv[1]);
-          return 1;
+          ReturnStatus = 1;
+          goto Done;
         }
         Options->VendId       = (UINT16) TempValue;
         Options->VendIdValid  = 1;
@@ -959,11 +963,13 @@ Returns:
         Status = AsciiStringToUint64(Argv[1], FALSE, &TempValue);
         if (EFI_ERROR (Status)) {
           Error (NULL, 0, 2000, "Invalid option value", "%s = %s", Argv[0], Argv[1]);
-          return 1;
+          ReturnStatus = 1;
+          goto Done;
         }
         if (TempValue >= 0x10000) {
           Error (NULL, 0, 2000, "Invalid option value", "Device Id %s out of range!", Argv[1]);
-          return 1;
+          ReturnStatus = 1;
+          goto Done;
         }
         Options->DevId      = (UINT16) TempValue;
         Options->DevIdValid = 1;
@@ -977,11 +983,13 @@ Returns:
         //
         if (Argv[1] == NULL || Argv[1][0] == '-') {
           Error (NULL, 0, 2000, "Invalid parameter", "Missing output file name with %s option!", Argv[0]);
-          return STATUS_ERROR;
+          ReturnStatus = STATUS_ERROR;
+          goto Done;
         }
         if (strlen (Argv[1]) > MAX_PATH - 1) {
           Error (NULL, 0, 2000, "Invalid parameter", "Output file name %s is too long!", Argv[1]);
-          return STATUS_ERROR;
+          ReturnStatus = STATUS_ERROR;
+          goto Done;
         }
         strncpy (Options->OutFileName, Argv[1], MAX_PATH - 1);
         Options->OutFileName[MAX_PATH - 1] = 0;
@@ -993,7 +1001,8 @@ Returns:
         // Help option
         //
         Usage ();
-        return STATUS_ERROR;
+        ReturnStatus = STATUS_ERROR;
+        goto Done;
       } else if (stricmp (Argv[0], "-b") == 0) {
         //
         // Specify binary files with -b
@@ -1021,11 +1030,13 @@ Returns:
         Status = AsciiStringToUint64(Argv[1], FALSE, &DebugLevel);
         if (EFI_ERROR (Status)) {
           Error (NULL, 0, 2000, "Invalid option value", "%s = %s", Argv[0], Argv[1]);
-          return 1;
+          ReturnStatus = 1;
+          goto Done;
         }
         if (DebugLevel > 9)  {
           Error (NULL, 0, 2000, "Invalid option value", "Debug Level range is 0-9, current input level is %d", Argv[1]);
-          return 1;
+          ReturnStatus = 1;
+          goto Done;
         }
         if (DebugLevel>=5 && DebugLevel<=9) {
           Options->Debug = TRUE;
@@ -1055,12 +1066,14 @@ Returns:
         Status = AsciiStringToUint64(Argv[1], FALSE, &TempValue);
         if (EFI_ERROR (Status)) {
           Error (NULL, 0, 2000, "Invalid option value", "%s = %s", Argv[0], Argv[1]);
-          return 1;
+          ReturnStatus = 1;
+          goto Done;
         }
         ClassCode = (UINT32) TempValue;
         if (ClassCode & 0xFF000000) {
           Error (NULL, 0, 2000, "Invalid parameter", "Class code %s out of range!", Argv[1]);
-          return STATUS_ERROR;
+          ReturnStatus = STATUS_ERROR;
+          goto Done;
         }
         if (FileList != NULL && FileList->ClassCode == 0) {
           FileList->ClassCode = ClassCode;
@@ -1076,12 +1089,14 @@ Returns:
         Status = AsciiStringToUint64(Argv[1], FALSE, &TempValue);
         if (EFI_ERROR (Status)) {
           Error (NULL, 0, 2000, "Invalid option value", "%s = %s", Argv[0], Argv[1]);
-          return 1;
+          ReturnStatus = 1;
+          goto Done;
         }
         CodeRevision = (UINT32) TempValue;
         if (CodeRevision & 0xFFFF0000) {
           Error (NULL, 0, 2000, "Invalid parameter", "Code revision %s out of range!", Argv[1]);
-          return STATUS_ERROR;
+          ReturnStatus = STATUS_ERROR;
+          goto Done;
         }
         if (FileList != NULL && FileList->CodeRevision == 0) {
           FileList->CodeRevision = (UINT16) CodeRevision;
@@ -1095,7 +1110,8 @@ Returns:
         mOptions.Pci23 = 1;
       } else {
         Error (NULL, 0, 2000, "Invalid parameter", "Invalid option specified: %s", Argv[0]);
-        return STATUS_ERROR;
+        ReturnStatus = STATUS_ERROR;
+        goto Done;
       }
     } else {
       //
@@ -1104,7 +1120,8 @@ Returns:
       //
       if ((FileFlags & (FILE_FLAG_BINARY | FILE_FLAG_EFI)) == 0) {
         Error (NULL, 0, 2000, "Invalid parameter", "Missing -e or -b with input file %s!", Argv[0]);
-        return STATUS_ERROR;
+        ReturnStatus = STATUS_ERROR;
+        goto Done;
       }
       //
       // Check Efi Option RomImage
@@ -1118,7 +1135,8 @@ Returns:
       FileList = (FILE_LIST *) malloc (sizeof (FILE_LIST));
       if (FileList == NULL) {
         Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!", NULL);
-        return STATUS_ERROR;
+        ReturnStatus = STATUS_ERROR;
+        goto Done;
       }
       
       //
@@ -1156,6 +1174,9 @@ Returns:
   //
   if (Options->FileList == NULL) {
     Error (NULL, 0, 2000, "Invalid parameter", "Missing input file name!");
+    //
+    // No memory allocation, return directly.
+    //
     return STATUS_ERROR;
   }
 
@@ -1165,16 +1186,27 @@ Returns:
   if (EfiRomFlag) {
     if (!Options->VendIdValid) {
       Error (NULL, 0, 2000, "Missing Vendor ID in command line", NULL);
-      return STATUS_ERROR;
+      ReturnStatus = STATUS_ERROR;
+      goto Done;
     }
   
     if (!Options->DevIdValid) {
       Error (NULL, 0, 2000, "Missing Device ID in command line", NULL);
-      return STATUS_ERROR;
+      ReturnStatus = STATUS_ERROR;
+      goto Done;
+    }
+  }
+
+Done:
+  if (ReturnStatus != 0) {
+    while (Options->FileList != NULL) {
+      FileList = Options->FileList->Next;
+      free (Options->FileList);
+      Options->FileList = FileList;
     }
   }
 
-  return 0;
+  return ReturnStatus;
 }
 
 static
-- 
1.9.5.msysgit.0



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

* [PATCH 31/52] BaseTools/GenFv: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (29 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 30/52] BaseTools/EfiRom: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 32/52] BaseTools/GenPage: " Hao Wu
                   ` (21 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index d7c650e..b653a61 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -1220,6 +1220,7 @@ Returns:
     if (CompareGuid ((EFI_GUID *) FileBuffer, &mFileGuidArray [Index1]) == 0) {
       Error (NULL, 0, 2000, "Invalid parameter", "the %dth file and %uth file have the same file GUID.", (unsigned) Index1 + 1, (unsigned) Index + 1);
       PrintGuid ((EFI_GUID *) FileBuffer);
+      free (FileBuffer);
       return EFI_INVALID_PARAMETER;
     }
   }
@@ -2626,7 +2627,7 @@ Returns:
   //
   Status = CalculateFvSize (&mFvDataInfo);
   if (EFI_ERROR (Status)) {
-    return Status;    
+    goto Finish;
   }
   VerboseMsg ("the generated FV image size is %u bytes", (unsigned) mFvDataInfo.Size);
   
@@ -2640,7 +2641,8 @@ Returns:
   //
   FvBufferHeader = malloc (FvImageSize + sizeof (UINT64));
   if (FvBufferHeader == NULL) {
-    return EFI_OUT_OF_RESOURCES;
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Finish;
   }
   FvImage = (UINT8 *) (((UINTN) FvBufferHeader + 7) & ~7);
 
@@ -2732,7 +2734,8 @@ Returns:
   FvMapFile = fopen (LongFilePath (FvMapName), "w");
   if (FvMapFile == NULL) {
     Error (NULL, 0, 0001, "Error opening file", FvMapName);
-    return EFI_ABORTED;
+    Status = EFI_ABORTED;
+    goto Finish;
   }
   
   //
@@ -2741,7 +2744,8 @@ Returns:
   FvReportFile = fopen (LongFilePath (FvReportName), "w");
   if (FvReportFile == NULL) {
     Error (NULL, 0, 0001, "Error opening file", FvReportName);
-    return EFI_ABORTED;
+    Status = EFI_ABORTED;
+    goto Finish;
   }
   //
   // record FV size information into FvMap file.
@@ -4259,6 +4263,7 @@ Returns:
 
   fwrite (CapBuffer, 1, CapSize, fpout);
   fclose (fpout);
+  free (CapBuffer);
   
   VerboseMsg ("The size of the generated capsule image is %u bytes", (unsigned) CapSize);
 
-- 
1.9.5.msysgit.0



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

* [PATCH 32/52] BaseTools/GenPage: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (30 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 31/52] BaseTools/GenFv: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 33/52] BaseTools/GenSec: " Hao Wu
                   ` (20 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenPage/GenPage.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/BaseTools/Source/C/GenPage/GenPage.c b/BaseTools/Source/C/GenPage/GenPage.c
index 5bf8bf8..16dc144 100644
--- a/BaseTools/Source/C/GenPage/GenPage.c
+++ b/BaseTools/Source/C/GenPage/GenPage.c
@@ -431,9 +431,11 @@ main (
   //
   result = GenBinPage (BaseMemory, InputFile, OutputFile);
   if (result < 0) {
+    free (BaseMemory);
     return STATUS_ERROR;
   }
 
+  free (BaseMemory);
   return 0;
 }
 
-- 
1.9.5.msysgit.0



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

* [PATCH 33/52] BaseTools/GenSec: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (31 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 32/52] BaseTools/GenPage: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 34/52] BaseTools/GenVtf: " Hao Wu
                   ` (19 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenSec/GenSec.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenSec/GenSec.c b/BaseTools/Source/C/GenSec/GenSec.c
index 583ff1d..2249dd6 100644
--- a/BaseTools/Source/C/GenSec/GenSec.c
+++ b/BaseTools/Source/C/GenSec/GenSec.c
@@ -902,6 +902,7 @@ Returns:
   }
 
   if (InputLength == 0) {
+    free (FileBuffer);
     Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s can't be zero", InputFileName);
     return EFI_NOT_FOUND;
   }
@@ -1215,7 +1216,7 @@ Returns:
         InputFileAlign = (UINT32 *) malloc (MAXIMUM_INPUT_FILE_NUM * sizeof (UINT32));
         if (InputFileAlign == NULL) {
           Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
-          return 1;
+          goto Finish;
         }
         memset (InputFileAlign, 1, MAXIMUM_INPUT_FILE_NUM * sizeof (UINT32));
       } else if (InputFileAlignNum % MAXIMUM_INPUT_FILE_NUM == 0) {
@@ -1226,7 +1227,7 @@ Returns:
 
         if (InputFileAlign == NULL) {
           Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
-          return 1;
+          goto Finish;
         }
         memset (&(InputFileAlign[InputFileNum]), 1, (MAXIMUM_INPUT_FILE_NUM * sizeof (UINT32)));
       }
@@ -1249,7 +1250,7 @@ Returns:
       InputFileName = (CHAR8 **) malloc (MAXIMUM_INPUT_FILE_NUM * sizeof (CHAR8 *));
       if (InputFileName == NULL) {
         Error (NULL, 0, 4001, "Resource", "memory cannot be allcoated");
-        return 1;
+        goto Finish;
       }
       memset (InputFileName, 0, (MAXIMUM_INPUT_FILE_NUM * sizeof (CHAR8 *)));
     } else if (InputFileNum % MAXIMUM_INPUT_FILE_NUM == 0) {
@@ -1263,7 +1264,7 @@ Returns:
 
       if (InputFileName == NULL) {
         Error (NULL, 0, 4001, "Resource", "memory cannot be allcoated");
-        return 1;
+        goto Finish;
       }
       memset (&(InputFileName[InputFileNum]), 0, (MAXIMUM_INPUT_FILE_NUM * sizeof (CHAR8 *)));
     }
-- 
1.9.5.msysgit.0



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

* [PATCH 34/52] BaseTools/GenVtf: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (32 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 33/52] BaseTools/GenSec: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 35/52] BaseTools/Split: Fix potential memory and resource leak Hao Wu
                   ` (18 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenVtf/GenVtf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/BaseTools/Source/C/GenVtf/GenVtf.c b/BaseTools/Source/C/GenVtf/GenVtf.c
index 8e75d04..392e4e0 100644
--- a/BaseTools/Source/C/GenVtf/GenVtf.c
+++ b/BaseTools/Source/C/GenVtf/GenVtf.c
@@ -1232,11 +1232,13 @@ Returns:
     Vtf1TotalSize += (UINT32) (FileSize + NumAdjustByte);
     Status = UpdateVtfBuffer (CompStartAddress, Buffer, FileSize, FIRST_VTF);
   } else {
+    free (Buffer);
     Error (NULL, 0, 2000,"Invalid Parameter", "There's component in second VTF so second BaseAddress and Size must be specified!");
     return EFI_INVALID_PARAMETER;
   }
 
   if (EFI_ERROR (Status)) {
+    free (Buffer);
     return EFI_ABORTED;
   }
 
@@ -1248,6 +1250,7 @@ Returns:
 
   CompFitPtr->CompAddress = CompStartAddress | IPF_CACHE_BIT;
   if ((FileSize % 16) != 0) {
+    free (Buffer);
     Error (NULL, 0, 2000, "Invalid parameter", "Binary FileSize must be a multiple of 16.");
     return EFI_INVALID_PARAMETER;
   }
@@ -1389,6 +1392,7 @@ Returns:
   PalFitPtr->CompAddress  = PalStartAddress | IPF_CACHE_BIT;
   //assert ((FileSize % 16) == 0);
   if ((FileSize % 16) != 0) {
+    free (Buffer);
     Error (NULL, 0, 2000, "Invalid parameter", "Binary FileSize must be a multiple of 16.");
     return EFI_INVALID_PARAMETER;
   }
-- 
1.9.5.msysgit.0



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

* [PATCH 35/52] BaseTools/Split: Fix potential memory and resource leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (33 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 34/52] BaseTools/GenVtf: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 36/52] BaseTools/TianoCompress: Fix potential memory leak Hao Wu
                   ` (17 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/Split/Split.c | 41 +++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/BaseTools/Source/C/Split/Split.c b/BaseTools/Source/C/Split/Split.c
index c6f547c..eb83d4c 100644
--- a/BaseTools/Source/C/Split/Split.c
+++ b/BaseTools/Source/C/Split/Split.c
@@ -223,14 +223,15 @@ Returns:
 --*/
 {
   EFI_STATUS    Status = EFI_SUCCESS;
+  INTN          ReturnStatus = STATUS_SUCCESS;
   FILE          *In;
   CHAR8         *InputFileName = NULL;
   CHAR8         *OutputDir = NULL;
   CHAR8         *OutFileName1 = NULL;
   CHAR8         *OutFileName2 = NULL;
   UINT64        SplitValue = (UINT64) -1;
-  FILE          *Out1;
-  FILE          *Out2;
+  FILE          *Out1 = NULL;
+  FILE          *Out2 = NULL;
   CHAR8         *OutName1 = NULL;
   CHAR8         *OutName2 = NULL;
   CHAR8         *CurrentDir = NULL;
@@ -366,7 +367,8 @@ Returns:
     OutName1 = (CHAR8*)malloc(strlen(InputFileName) + 16);
     if (OutName1 == NULL) {
       Warning (NULL, 0, 0, NULL, "Memory Allocation Fail.");
-      return STATUS_ERROR;
+      ReturnStatus = STATUS_ERROR;
+      goto Finish;
     }
     strcpy (OutName1, InputFileName);
     strcat (OutName1, "1");
@@ -377,7 +379,8 @@ Returns:
     OutName2 = (CHAR8*)malloc(strlen(InputFileName) + 16);
     if (OutName2 == NULL) {
       Warning (NULL, 0, 0, NULL, "Memory Allocation Fail.");
-      return STATUS_ERROR;
+      ReturnStatus = STATUS_ERROR;
+      goto Finish;
     }
     strcpy (OutName2, InputFileName);
     strcat (OutName2, "2");
@@ -389,20 +392,23 @@ Returns:
     //OutputDirSpecified = TRUE;
     if (chdir(OutputDir) != 0) {
       Warning (NULL, 0, 0, NULL, "Change dir to OutputDir Fail.");
-      return STATUS_ERROR;
+      ReturnStatus = STATUS_ERROR;
+      goto Finish;
     }
   }
 
   CurrentDir = (CHAR8*)getcwd((CHAR8*)0, 0);
   if (EFI_ERROR(CreateDir(&OutFileName1))) {
       Error (OutFileName1, 0, 5, "Create Dir for File1 Fail.", NULL);
-      return STATUS_ERROR;
+      ReturnStatus = STATUS_ERROR;
+      goto Finish;
   }
   chdir(CurrentDir);
 
   if (EFI_ERROR(CreateDir(&OutFileName2))) {
       Error (OutFileName2, 0, 5, "Create Dir for File2 Fail.", NULL);
-      return STATUS_ERROR;
+      ReturnStatus = STATUS_ERROR;
+      goto Finish;
   }
   chdir(CurrentDir);
   free(CurrentDir);
@@ -411,14 +417,16 @@ Returns:
   if (Out1 == NULL) {
     // ("Unable to open file \"%s\"\n", OutFileName1);
     Error (OutFileName1, 0, 1, "File open failure", NULL);
-    return STATUS_ERROR;
+    ReturnStatus = STATUS_ERROR;
+    goto Finish;
   }
 
   Out2 = fopen (LongFilePath (OutFileName2), "wb");
   if (Out2 == NULL) {
     // ("Unable to open file \"%s\"\n", OutFileName2);
     Error (OutFileName2, 0, 1, "File open failure", NULL);
-    return STATUS_ERROR;
+    ReturnStatus = STATUS_ERROR;
+    goto Finish;
   }
 
   for (Index = 0; Index < SplitValue; Index++) {
@@ -439,15 +447,22 @@ Returns:
     fputc (CharC, Out2);
   }
 
+Finish:
   if (OutName1 != NULL) {
     free(OutName1);
   }
   if (OutName2 != NULL) {
     free(OutName2);
   }
-  fclose (In);
-  fclose (Out1);
-  fclose (Out2);
+  if (In != NULL) {
+    fclose (In);
+  }
+  if (Out1 != NULL) {
+    fclose (Out1);
+  }
+  if (Out2 != NULL) {
+    fclose (Out2);
+  }
 
-  return STATUS_SUCCESS;
+  return ReturnStatus;
 }
-- 
1.9.5.msysgit.0



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

* [PATCH 36/52] BaseTools/TianoCompress: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (34 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 35/52] BaseTools/Split: Fix potential memory and resource leak Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 37/52] BaseTools/VfrCompile: " Hao Wu
                   ` (16 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index 93cb6c3..44dbccf 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -1906,7 +1906,7 @@ Returns:
     FileBuffer = (UINT8 *) malloc (InputLength);
     if (FileBuffer == NULL) {
       Error (NULL, 0, 4001, "Resource:", "Memory cannot be allocated!");
-      return 1;
+      goto ERROR;
     }
 
     Status = GetFileContents (
@@ -1917,8 +1917,8 @@ Returns:
   }
 
   if (EFI_ERROR(Status)) {
-    free(FileBuffer);
-    return 1;
+    Error (NULL, 0, 0004, "Error getting contents of file: %s", InputFileName);
+    goto ERROR;
   }
 
   if (OutputFileName == NULL) {
-- 
1.9.5.msysgit.0



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

* [PATCH 37/52] BaseTools/VfrCompile: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (35 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 36/52] BaseTools/TianoCompress: Fix potential memory leak Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 38/52] BaseTools/VolInfo: " Hao Wu
                   ` (15 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
index d06c1bc..ec73529 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
@@ -113,6 +113,7 @@ CFormPkg::CFormPkg (
   }
   BufferStart = new CHAR8[BufferSize];
   if (BufferStart == NULL) {
+    delete Node;
     return;
   }
   BufferEnd   = BufferStart + BufferSize;
-- 
1.9.5.msysgit.0



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

* [PATCH 38/52] BaseTools/VolInfo: Fix potential memory leak
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (36 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 37/52] BaseTools/VfrCompile: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 39/52] BaseTools/EfiRom: Fix file handles not being closed Hao Wu
                   ` (14 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VolInfo/VolInfo.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c b/BaseTools/Source/C/VolInfo/VolInfo.c
index 5da6582..312d332 100644
--- a/BaseTools/Source/C/VolInfo/VolInfo.c
+++ b/BaseTools/Source/C/VolInfo/VolInfo.c
@@ -258,6 +258,14 @@ Returns:
       continue;
     }
     if ((stricmp (argv[0], "--hash") == 0)) {
+      if (EnableHash == TRUE) {
+        //
+        // --hash already given in the option, ignore this one
+        //
+        argc --;
+        argv ++;
+        continue;
+      }
       EnableHash = TRUE;
       OpenSslCommand = "openssl";
       OpenSslEnv = getenv("OPENSSL_PATH");
@@ -1784,8 +1792,14 @@ Returns:
         }
 
         ScratchBuffer       = malloc (ScratchSize);
+        if (ScratchBuffer == NULL) {
+          Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+          return EFI_OUT_OF_RESOURCES;
+        }
         UncompressedBuffer  = malloc (UncompressedLength);
-        if ((ScratchBuffer == NULL) || (UncompressedBuffer == NULL)) {
+        if (UncompressedBuffer == NULL) {
+          free (ScratchBuffer);
+          Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
           return EFI_OUT_OF_RESOURCES;
         }
         Status = DecompressFunction (
-- 
1.9.5.msysgit.0



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

* [PATCH 39/52] BaseTools/EfiRom: Fix file handles not being closed
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (37 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 38/52] BaseTools/VolInfo: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 40/52] BaseTools/GenBootSector: " Hao Wu
                   ` (13 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/EfiRom/EfiRom.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/EfiRom/EfiRom.c b/BaseTools/Source/C/EfiRom/EfiRom.c
index e862989..afb53cd 100644
--- a/BaseTools/Source/C/EfiRom/EfiRom.c
+++ b/BaseTools/Source/C/EfiRom/EfiRom.c
@@ -176,9 +176,6 @@ Returns:
 
 BailOut:
   if (Status == STATUS_SUCCESS) {
-    if (FptrOut != NULL) {
-      fclose (FptrOut);
-    }
     //
     // Clean up our file list
     //
@@ -189,6 +186,10 @@ BailOut:
     }
   }
 
+  if (FptrOut != NULL) {
+    fclose (FptrOut);
+  }
+
   if (mOptions.Verbose) {
     VerboseMsg("%s tool done with return code is 0x%x.\n", UTILITY_NAME, GetUtilityStatus ());
   }
-- 
1.9.5.msysgit.0



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

* [PATCH 40/52] BaseTools/GenBootSector: Fix file handles not being closed
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (38 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 39/52] BaseTools/EfiRom: Fix file handles not being closed Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 41/52] BaseTools/GenCrc32: " Hao Wu
                   ` (12 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenBootSector/GenBootSector.c | 35 ++++++++++++++++--------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/BaseTools/Source/C/GenBootSector/GenBootSector.c b/BaseTools/Source/C/GenBootSector/GenBootSector.c
index efdfcd2..2ab2d2a 100644
--- a/BaseTools/Source/C/GenBootSector/GenBootSector.c
+++ b/BaseTools/Source/C/GenBootSector/GenBootSector.c
@@ -201,6 +201,7 @@ Return:
     //
     // Only care about the disk.
     //
+    CloseHandle(VolumeHandle);
     return FALSE;
   } else{
     DriveInfo->DiskNumber = StorageDeviceNumber.DeviceNumber;
@@ -437,8 +438,8 @@ ProcessBsOrMbr (
   BYTE              DiskPartitionBackup[0x200] = {0};
   DWORD             BytesReturn;
   INT               DrvNumOffset;
-  HANDLE            InputHandle;
-  HANDLE            OutputHandle;
+  HANDLE            InputHandle = INVALID_HANDLE_VALUE;
+  HANDLE            OutputHandle = INVALID_HANDLE_VALUE;
   ERROR_STATUS      Status;
   DWORD             InputDbrOffset;
   DWORD             OutputDbrOffset;
@@ -448,7 +449,7 @@ ProcessBsOrMbr (
   //
   Status =  GetFileHandle(InputInfo, ProcessMbr, &InputHandle, &InputDbrOffset);
   if (Status != ErrorSuccess) {
-    return Status;
+    goto Done;
   }
 
   //
@@ -456,14 +457,15 @@ ProcessBsOrMbr (
   //
   Status = GetFileHandle(OutputInfo, ProcessMbr, &OutputHandle, &OutputDbrOffset);
   if (Status != ErrorSuccess) {
-    return Status;
+    goto Done;
   }
 
   //
   // Read boot sector from source disk/file
   // 
   if (!ReadFile (InputHandle, DiskPartition, 0x200, &BytesReturn, NULL)) {
-    return ErrorFileReadWrite;
+    Status = ErrorFileReadWrite;
+    goto Done;
   }
 
   if (InputInfo->Type == PathUsb) {
@@ -473,7 +475,8 @@ ProcessBsOrMbr (
       //
       DrvNumOffset = GetDrvNumOffset (DiskPartition);
       if (DrvNumOffset == -1) {
-        return ErrorFatType;
+        Status = ErrorFatType;
+        goto Done;
       }
       //
       // Some legacy BIOS require 0x80 discarding MBR.
@@ -495,7 +498,8 @@ ProcessBsOrMbr (
       // Use original partition table
       //
       if (!ReadFile (OutputHandle, DiskPartitionBackup, 0x200, &BytesReturn, NULL)) {
-        return ErrorFileReadWrite;
+        Status = ErrorFileReadWrite;
+        goto Done;
       }
       memcpy (DiskPartition + 0x1BE, DiskPartitionBackup + 0x1BE, 0x40);
       SetFilePointer (OutputHandle, 0, NULL, FILE_BEGIN);
@@ -507,13 +511,19 @@ ProcessBsOrMbr (
   // Write boot sector to taget disk/file
   // 
   if (!WriteFile (OutputHandle, DiskPartition, 0x200, &BytesReturn, NULL)) {
-    return ErrorFileReadWrite;
+    Status = ErrorFileReadWrite;
+    goto Done;
   }
 
-  CloseHandle (InputHandle);
-  CloseHandle (OutputHandle);
+Done:
+  if (InputHandle != INVALID_HANDLE_VALUE) {
+    CloseHandle (InputHandle);
+  }
+  if (OutputHandle != INVALID_HANDLE_VALUE) {
+    CloseHandle (OutputHandle);
+  }
 
-  return ErrorSuccess;
+  return Status;
 }
 
 void
@@ -630,7 +640,8 @@ GetPathInfo (
     if (f == NULL) {
       fprintf (stderr, "error E2003: File was not provided!\n");
       return ErrorPath;
-    }  
+    }
+    fclose (f);
   }
   PathInfo->Type = PathFile;
   strcpy(PathInfo->PhysicalPath, PathInfo->Path);
-- 
1.9.5.msysgit.0



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

* [PATCH 41/52] BaseTools/GenCrc32: Fix file handles not being closed
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (39 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 40/52] BaseTools/GenBootSector: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 42/52] BaseTools/GenFv: " Hao Wu
                   ` (11 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenCrc32/GenCrc32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenCrc32/GenCrc32.c b/BaseTools/Source/C/GenCrc32/GenCrc32.c
index ad8cf57..89a0e3b 100644
--- a/BaseTools/Source/C/GenCrc32/GenCrc32.c
+++ b/BaseTools/Source/C/GenCrc32/GenCrc32.c
@@ -1,7 +1,7 @@
 /** @file
 Calculate Crc32 value and Verify Crc32 value for input data.
 
-Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2007 - 2016, 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        
@@ -289,6 +289,7 @@ Returns:
   FileBuffer = (UINT8 *) malloc (FileSize);
   if (FileBuffer == NULL) {
     Error (NULL, 0, 4001, "Resource", "memory cannot be allcoated!");
+    fclose (InFile);
     goto Finish;
   }
   
-- 
1.9.5.msysgit.0



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

* [PATCH 42/52] BaseTools/GenFv: Fix file handles not being closed
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (40 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 41/52] BaseTools/GenCrc32: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 43/52] BaseTools/GenVtf: " Hao Wu
                   ` (10 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index b653a61..54a9d07 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -1161,6 +1161,7 @@ Returns:
   //
   FileBuffer = malloc (FileSize);
   if (FileBuffer == NULL) {
+    fclose (NewFile);
     Error (NULL, 0, 4001, "Resouce", "memory cannot be allocated!");
     return EFI_OUT_OF_RESOURCES;
   }
@@ -3506,6 +3507,7 @@ Returns:
           PeFileSize = _filelength (fileno (PeFile));
           PeFileBuffer = (UINT8 *) malloc (PeFileSize);
           if (PeFileBuffer == NULL) {
+            fclose (PeFile);
             Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase of %s", FileName);
             return EFI_OUT_OF_RESOURCES;
           }
@@ -3761,6 +3763,7 @@ Returns:
         PeFileSize = _filelength (fileno (PeFile));
         PeFileBuffer = (UINT8 *) malloc (PeFileSize);
         if (PeFileBuffer == NULL) {
+          fclose (PeFile);
           Error (NULL, 0, 4001, "Resource", "memory cannot be allocated on rebase of %s", FileName);
           return EFI_OUT_OF_RESOURCES;
         }
-- 
1.9.5.msysgit.0



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

* [PATCH 43/52] BaseTools/GenVtf: Fix file handles not being closed
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (41 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 42/52] BaseTools/GenFv: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 44/52] BaseTools/LzmaCompress: " Hao Wu
                   ` (9 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenVtf/GenVtf.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/BaseTools/Source/C/GenVtf/GenVtf.c b/BaseTools/Source/C/GenVtf/GenVtf.c
index 392e4e0..85c771d 100644
--- a/BaseTools/Source/C/GenVtf/GenVtf.c
+++ b/BaseTools/Source/C/GenVtf/GenVtf.c
@@ -1164,6 +1164,7 @@ Returns:
 
   if (VtfInfo->PreferredSize) {
     if (FileSize > VtfInfo->CompSize) {
+      fclose (Fp);
       Error (NULL, 0, 2000, "Invalid parameter", "The component size is more than specified size.");
       return EFI_ABORTED;
     }
@@ -1173,6 +1174,7 @@ Returns:
 
   Buffer = malloc ((UINTN) FileSize);
   if (Buffer == NULL) {
+    fclose (Fp);
     return EFI_OUT_OF_RESOURCES;
   }
   memset (Buffer, 0, (UINTN) FileSize);
@@ -1342,6 +1344,7 @@ Returns:
 
   FileSize = _filelength (fileno (Fp));
   if (FileSize < 64) {
+    fclose (Fp);
     Error (NULL, 0, 2000, "Invalid parameter", "PAL_A bin header is 64 bytes, so the Bin size must be larger than 64 bytes!");
     return EFI_INVALID_PARAMETER;
   }
@@ -1350,6 +1353,7 @@ Returns:
 
   if (VtfInfo->PreferredSize) {
     if (FileSize > VtfInfo->CompSize) {
+      fclose (Fp);
       Error (NULL, 0, 2000, "Invalid parameter", "The PAL_A Size is more than the specified size.");
       return EFI_ABORTED;
     }
@@ -1359,6 +1363,7 @@ Returns:
 
   Buffer = malloc ((UINTN) FileSize);
   if (Buffer == NULL) {
+    fclose (Fp);
     return EFI_OUT_OF_RESOURCES;
   }
   memset (Buffer, 0, (UINTN) FileSize);
@@ -1775,11 +1780,13 @@ Returns:
   FileSize = _filelength (fileno (Fp));
 
   if (FileSize > 16) {
+    fclose (Fp);
     return EFI_ABORTED;
   }
 
   Buffer = malloc (FileSize);
   if (Buffer == NULL) {
+    fclose (Fp);
     return EFI_OUT_OF_RESOURCES;
   }
 
@@ -2548,6 +2555,12 @@ Returns:
       // Get the input VTF file name
       //
       VtfFileName = argv[Index+1];
+      if (VtfFP != NULL) {
+        //
+        // VTF file name has been given previously, override with the new value
+        //
+        fclose (VtfFP);
+      }
       VtfFP = fopen (LongFilePath (VtfFileName), "rb");
       if (VtfFP == NULL) {
         Error (NULL, 0, 0001, "Error opening file", VtfFileName);
-- 
1.9.5.msysgit.0



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

* [PATCH 44/52] BaseTools/LzmaCompress: Fix file handles not being closed
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (42 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 43/52] BaseTools/GenVtf: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 45/52] BaseTools/TianoCompress: " Hao Wu
                   ` (8 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/LzmaCompress/LzmaCompress.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/LzmaCompress/LzmaCompress.c b/BaseTools/Source/C/LzmaCompress/LzmaCompress.c
index 1de07a3..ee6e1fe 100644
--- a/BaseTools/Source/C/LzmaCompress/LzmaCompress.c
+++ b/BaseTools/Source/C/LzmaCompress/LzmaCompress.c
@@ -5,7 +5,7 @@
     LzmaUtil.c -- Test application for LZMA compression
     2008-11-23 : Igor Pavlov : Public domain
 
-  Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2016, 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
@@ -333,8 +333,10 @@ int main2(int numArgs, const char *args[], char *rs)
   if (InFile_Open(&inStream.file, inputFile) != 0)
     return PrintError(rs, "Can not open input file");
 
-  if (OutFile_Open(&outStream.file, outputFile) != 0)
+  if (OutFile_Open(&outStream.file, outputFile) != 0) {
+    File_Close(&inStream.file);
     return PrintError(rs, "Can not open output file");
+  }
 
   File_GetLength(&inStream.file, &fileSize);
 
-- 
1.9.5.msysgit.0



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

* [PATCH 45/52] BaseTools/TianoCompress: Fix file handles not being closed
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (43 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 44/52] BaseTools/LzmaCompress: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 46/52] BaseTools/VolInfo: " Hao Wu
                   ` (7 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/TianoCompress/TianoCompress.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/TianoCompress/TianoCompress.c b/BaseTools/Source/C/TianoCompress/TianoCompress.c
index 44dbccf..f810511 100644
--- a/BaseTools/Source/C/TianoCompress/TianoCompress.c
+++ b/BaseTools/Source/C/TianoCompress/TianoCompress.c
@@ -1764,6 +1764,8 @@ Returns:
   InputLength = 0;
   InputFileName = NULL;
   OutputFileName = NULL;
+  InputFile = NULL;
+  OutputFile = NULL;
   DstSize=0;
   DebugLevel = 0;
   DebugMode = FALSE;
@@ -1927,9 +1929,6 @@ Returns:
   OutputFile = fopen (LongFilePath (OutputFileName), "wb");
   if (OutputFile == NULL) {
     Error (NULL, 0, 0001, "Error opening output file for writing", OutputFileName);
-    if (InputFile != NULL) {
-      fclose (InputFile);
-    }
     goto ERROR;
   }
     
@@ -1962,6 +1961,8 @@ Returns:
   }
 
   fwrite(OutBuffer,(size_t)DstSize, 1, OutputFile);
+  fclose(OutputFile);
+  fclose(InputFile);
   free(Scratch);
   free(FileBuffer);
   free(OutBuffer);
@@ -1999,6 +2000,8 @@ Returns:
   }
 
   fwrite(OutBuffer, (size_t)(Scratch->mOrigSize), 1, OutputFile);
+  fclose(OutputFile);
+  fclose(InputFile);
   free(Scratch);
   free(FileBuffer);
   free(OutBuffer);
@@ -2021,6 +2024,12 @@ ERROR:
       DebugMsg(UTILITY_NAME, 0, DebugLevel, "Decoding Error\n", NULL);
     }
   }
+  if (OutputFile != NULL) {
+    fclose(OutputFile);
+  }
+  if (InputFile != NULL) {
+    fclose (InputFile);
+  }
   if (Scratch != NULL) {
     free(Scratch);
   }
-- 
1.9.5.msysgit.0



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

* [PATCH 46/52] BaseTools/VolInfo: Fix file handles not being closed
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (44 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 45/52] BaseTools/TianoCompress: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 47/52] BaseTools/GenVtf: Fix potential buffer overflow in scanf functions Hao Wu
                   ` (6 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VolInfo/VolInfo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c b/BaseTools/Source/C/VolInfo/VolInfo.c
index 312d332..07840bf 100644
--- a/BaseTools/Source/C/VolInfo/VolInfo.c
+++ b/BaseTools/Source/C/VolInfo/VolInfo.c
@@ -2191,6 +2191,7 @@ Returns:
     //
     GPtr = malloc (sizeof (GUID_TO_BASENAME));
     if (GPtr == NULL) {
+      fclose (Fptr);
       return EFI_OUT_OF_RESOURCES;
     }
 
-- 
1.9.5.msysgit.0



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

* [PATCH 47/52] BaseTools/GenVtf: Fix potential buffer overflow in scanf functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (45 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 46/52] BaseTools/VolInfo: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 48/52] BaseTools/VolInfo: " Hao Wu
                   ` (5 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

String width is not specified for '%s' specifier in the format string for
scanf functions. This can result in buffer overflows.

This commit now specifies the string length for '%s' in format strings
according to the size of receiving buffers.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/GenVtf/GenVtf.c | 82 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenVtf/GenVtf.c b/BaseTools/Source/C/GenVtf/GenVtf.c
index 85c771d..9c485d3 100644
--- a/BaseTools/Source/C/GenVtf/GenVtf.c
+++ b/BaseTools/Source/C/GenVtf/GenVtf.c
@@ -1045,6 +1045,7 @@ Arguments:
 Returns:
 
   EFI_INVALID_PARAMETER  - The parameter is invalid
+  EFI_OUT_OF_RESOURCES   - Resource can not be allocated
   EFI_SUCCESS            - The function completed successfully
 
 --*/
@@ -1062,6 +1063,8 @@ Returns:
   CHAR8   Buff4[10];
   CHAR8   Buff5[10];
   CHAR8   Token[50];
+  CHAR8   *FormatString;
+  INTN    FormatLength;
 
   Fp = fopen (LongFilePath (VtfInfo->CompSymName), "rb");
 
@@ -1070,10 +1073,47 @@ Returns:
     return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // Generate the format string for fscanf
+  //
+  FormatLength = snprintf (
+                   NULL,
+                   0,
+                   "%%%us %%%us %%%us %%%us %%%us %%%us %%%us",
+                   (unsigned) sizeof (Buff1) - 1,
+                   (unsigned) sizeof (Buff2) - 1,
+                   (unsigned) sizeof (OffsetStr) - 1,
+                   (unsigned) sizeof (Buff3) - 1,
+                   (unsigned) sizeof (Buff4) - 1,
+                   (unsigned) sizeof (Buff5) - 1,
+                   (unsigned) sizeof (Token) - 1
+                   ) + 1;
+
+  FormatString = (CHAR8 *) malloc (FormatLength);
+  if (FormatString == NULL) {
+    fclose (Fp);
+
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  snprintf (
+    FormatString,
+    FormatLength,
+    "%%%us %%%us %%%us %%%us %%%us %%%us %%%us",
+    (unsigned) sizeof (Buff1) - 1,
+    (unsigned) sizeof (Buff2) - 1,
+    (unsigned) sizeof (OffsetStr) - 1,
+    (unsigned) sizeof (Buff3) - 1,
+    (unsigned) sizeof (Buff4) - 1,
+    (unsigned) sizeof (Buff5) - 1,
+    (unsigned) sizeof (Token) - 1
+    );
+
   while (fgets (Buff, sizeof (Buff), Fp) != NULL) {
     fscanf (
       Fp,
-      "%s %s %s %s %s %s %s",
+      FormatString,
       Buff1,
       Buff2,
       OffsetStr,
@@ -1096,6 +1136,10 @@ Returns:
 
   memcpy ((VOID *) RelativeAddress, (VOID *) CompStartAddress, sizeof (UINT64));
 
+  if (FormatString != NULL) {
+    free (FormatString);
+  }
+
   if (Fp != NULL) {
     fclose (Fp);
   }
@@ -2198,6 +2242,8 @@ Returns:
   CHAR8   Section[MAX_LONG_FILE_PATH];
   CHAR8   Token[MAX_LONG_FILE_PATH];
   CHAR8   BaseToken[MAX_LONG_FILE_PATH];
+  CHAR8   *FormatString;
+  INTN    FormatLength;
   UINT64  TokenAddress;
   long    StartLocation;
 
@@ -2276,6 +2322,37 @@ Returns:
   }
 
   //
+  // Generate the format string for fscanf
+  //
+  FormatLength = snprintf (
+                   NULL,
+                   0,
+                   "%%%us | %%%us | %%%us | %%%us\n",
+                   (unsigned) sizeof (Type) - 1,
+                   (unsigned) sizeof (Address) - 1,
+                   (unsigned) sizeof (Section) - 1,
+                   (unsigned) sizeof (Token) - 1
+                   ) + 1;
+
+  FormatString = (CHAR8 *) malloc (FormatLength);
+  if (FormatString == NULL) {
+    fclose (SourceFile);
+    fclose (DestFile);
+    Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+    return EFI_ABORTED;
+  }
+
+  snprintf (
+    FormatString,
+    FormatLength,
+    "%%%us | %%%us | %%%us | %%%us\n",
+    (unsigned) sizeof (Type) - 1,
+    (unsigned) sizeof (Address) - 1,
+    (unsigned) sizeof (Section) - 1,
+    (unsigned) sizeof (Token) - 1
+    );
+
+  //
   // Read in the file
   //
   while (feof (SourceFile) == 0) {
@@ -2283,7 +2360,7 @@ Returns:
     //
     // Read a line
     //
-    if (fscanf (SourceFile, "%s | %s | %s | %s\n", Type, Address, Section, Token) == 4) {
+    if (fscanf (SourceFile, FormatString, Type, Address, Section, Token) == 4) {
 
       //
       // Get the token address
@@ -2306,6 +2383,7 @@ Returns:
     }
   }
 
+  free (FormatString);
   fclose (SourceFile);
   fclose (DestFile);
   return EFI_SUCCESS;
-- 
1.9.5.msysgit.0



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

* [PATCH 48/52] BaseTools/VolInfo: Fix potential buffer overflow in scanf functions
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (46 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 47/52] BaseTools/GenVtf: Fix potential buffer overflow in scanf functions Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 49/52] BaseTools/VfrCompile: Explicitly state format string for DebugMsg() Hao Wu
                   ` (4 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

String width is not specified for '%s' specifier in the format string for
scanf functions. This can result in buffer overflows.

This commit now specifies the string length for '%s' in format strings
according to the size of receiving buffers.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VolInfo/VolInfo.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c b/BaseTools/Source/C/VolInfo/VolInfo.c
index 07840bf..5285acd 100644
--- a/BaseTools/Source/C/VolInfo/VolInfo.c
+++ b/BaseTools/Source/C/VolInfo/VolInfo.c
@@ -2178,6 +2178,8 @@ Returns:
 {
   FILE              *Fptr;
   CHAR8             Line[MAX_LINE_LEN];
+  CHAR8             *FormatString;
+  INTN              FormatLength;
   GUID_TO_BASENAME  *GPtr;
 
   if ((Fptr = fopen (LongFilePath (FileName), "r")) == NULL) {
@@ -2185,18 +2187,44 @@ Returns:
     return EFI_DEVICE_ERROR;
   }
 
+  //
+  // Generate the format string for fscanf
+  //
+  FormatLength = snprintf (
+                   NULL,
+                   0,
+                   "%%%us %%%us",
+                   (unsigned) sizeof (GPtr->Guid) - 1,
+                   (unsigned) sizeof (GPtr->BaseName) - 1
+                   ) + 1;
+
+  FormatString = (CHAR8 *) malloc (FormatLength);
+  if (FormatString == NULL) {
+    fclose (Fptr);
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  snprintf (
+    FormatString,
+    FormatLength,
+    "%%%us %%%us",
+    (unsigned) sizeof (GPtr->Guid) - 1,
+    (unsigned) sizeof (GPtr->BaseName) - 1
+    );
+
   while (fgets (Line, sizeof (Line), Fptr) != NULL) {
     //
     // Allocate space for another guid/basename element
     //
     GPtr = malloc (sizeof (GUID_TO_BASENAME));
     if (GPtr == NULL) {
+      free (FormatString);
       fclose (Fptr);
       return EFI_OUT_OF_RESOURCES;
     }
 
     memset ((char *) GPtr, 0, sizeof (GUID_TO_BASENAME));
-    if (sscanf (Line, "%s %s", GPtr->Guid, GPtr->BaseName) == 2) {
+    if (sscanf (Line, FormatString, GPtr->Guid, GPtr->BaseName) == 2) {
       GPtr->Next        = mGuidBaseNameList;
       mGuidBaseNameList = GPtr;
     } else {
@@ -2207,6 +2235,7 @@ Returns:
     }
   }
 
+  free (FormatString);
   fclose (Fptr);
   return EFI_SUCCESS;
 }
-- 
1.9.5.msysgit.0



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

* [PATCH 49/52] BaseTools/VfrCompile: Explicitly state format string for DebugMsg()
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (47 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 48/52] BaseTools/VolInfo: " Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 50/52] BaseTools/VolInfo: Use hard-coded format string for calls to sprintf() Hao Wu
                   ` (3 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

For calls to API DebugMsg(), explicitly state format string as "%s" when
the given variable list is a sting.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index 0e5c7c6..1524584 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -134,7 +134,7 @@ CVfrCompiler::OptionInitialization (
           strcat (mOptions.OutputDirectory, "\\");
         }
       }
-      DebugMsg (NULL, 0, 9, (CHAR8 *) "Output Directory", mOptions.OutputDirectory);
+      DebugMsg (NULL, 0, 9, (CHAR8 *) "Output Directory", (CHAR8 *) "%s", mOptions.OutputDirectory);
     } else if (stricmp(Argv[Index], "-b") == 0 || stricmp(Argv[Index], "--create-ifr-package") == 0 || stricmp(Argv[Index], "-ibin") == 0) {
       mOptions.CreateIfrPkgFile = TRUE;
     } else if (stricmp(Argv[Index], "-n") == 0 || stricmp(Argv[Index], "--no-pre-processing") == 0 || stricmp(Argv[Index], "-nopp") == 0) {
@@ -156,7 +156,7 @@ CVfrCompiler::OptionInitialization (
         goto Fail;
       }
       gCVfrStringDB.SetStringFileName(Argv[Index]);
-      DebugMsg (NULL, 0, 9, (CHAR8 *) "Input string file path", Argv[Index]);
+      DebugMsg (NULL, 0, 9, (CHAR8 *) "Input string file path", (CHAR8 *) "%s", Argv[Index]);
     } else if ((stricmp (Argv[Index], "-g") == 0) || (stricmp (Argv[Index], "--guid") == 0)) {
       Index++;
       Status = StringToGuid (Argv[Index], &mOptions.OverrideClassGuid);
-- 
1.9.5.msysgit.0



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

* [PATCH 50/52] BaseTools/VolInfo: Use hard-coded format string for calls to sprintf()
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (48 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 49/52] BaseTools/VfrCompile: Explicitly state format string for DebugMsg() Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 51/52] BaseTools/VfrCompile/Pccts: Add virtual destructor for class DLGInputStream Hao Wu
                   ` (2 subsequent siblings)
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu

For calls to API sprintf(), use hard-coded format string instead of a
local variable.

This helps to prevent the format string from being changed accidentally,
which may lead to potential buffer overflows.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VolInfo/VolInfo.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c b/BaseTools/Source/C/VolInfo/VolInfo.c
index 5285acd..7ecfb7f 100644
--- a/BaseTools/Source/C/VolInfo/VolInfo.c
+++ b/BaseTools/Source/C/VolInfo/VolInfo.c
@@ -1599,7 +1599,6 @@ Returns:
   CHAR8               *ExtractionTool;
   CHAR8               *ToolInputFile;
   CHAR8               *ToolOutputFile;
-  CHAR8               *SystemCommandFormatString;
   CHAR8               *SystemCommand;
   EFI_GUID            *EfiGuid;
   UINT16              DataOffset;
@@ -1659,9 +1658,8 @@ Returns:
           SectionLength - SectionHeaderLen
           );
 
-        SystemCommandFormatString = "%s sha1 -out %s %s";
         SystemCommand = malloc (
-          strlen (SystemCommandFormatString) +
+          strlen ("%s sha1 -out %s %s") +
           strlen (OpenSslPath) +
           strlen (ToolInputFileName) +
           strlen (ToolOutputFileName) +
@@ -1673,7 +1671,7 @@ Returns:
         }
         sprintf (
           SystemCommand,
-          SystemCommandFormatString,
+          "%s sha1 -out %s %s",
           OpenSslPath,
           ToolOutputFileName,
           ToolInputFileName
@@ -1891,9 +1889,8 @@ Returns:
         //
         // Construction 'system' command string
         //
-        SystemCommandFormatString = "%s -d -o %s %s";
         SystemCommand = malloc (
-          strlen (SystemCommandFormatString) +
+          strlen ("%s -d -o %s %s") +
           strlen (ExtractionTool) +
           strlen (ToolInputFile) +
           strlen (ToolOutputFile) +
@@ -1909,7 +1906,7 @@ Returns:
         }
         sprintf (
           SystemCommand,
-          SystemCommandFormatString,
+          "%s -d -o %s %s",
           ExtractionTool,
           ToolOutputFile,
           ToolInputFile
-- 
1.9.5.msysgit.0



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

* [PATCH 51/52] BaseTools/VfrCompile/Pccts: Add virtual destructor for class DLGInputStream
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (49 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 50/52] BaseTools/VolInfo: Use hard-coded format string for calls to sprintf() Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-12 12:20 ` [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void Hao Wu
  2016-10-17  7:45 ` [PATCH 00/52] Resolve issues for C source codes in BaseTools Gao, Liming
  52 siblings, 0 replies; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

Class DLGInputStream defined in DLexerBase.h has a virtual method but no
virtual destructor.

This commit add an empty virtual destructor to avoid potential
memory/resource leak when an object of a class derived from class
DLGInputStream is deleted through a pointer to the DLGInputStream class.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h b/BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h
index 667ecfd..b9ca311 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h
+++ b/BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h
@@ -57,6 +57,7 @@ public:
 class DllExportPCCTS DLGInputStream {
 public:
 	virtual int nextChar() = 0;
+    virtual ~DLGInputStream() {};
 };
 
 /* Predefined char stream: Input from FILE */
-- 
1.9.5.msysgit.0



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

* [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (50 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 51/52] BaseTools/VfrCompile/Pccts: Add virtual destructor for class DLGInputStream Hao Wu
@ 2016-10-12 12:20 ` Hao Wu
  2016-10-18  1:12   ` Dong, Eric
  2016-10-17  7:45 ` [PATCH 00/52] Resolve issues for C source codes in BaseTools Gao, Liming
  52 siblings, 1 reply; 55+ messages in thread
From: Hao Wu @ 2016-10-12 12:20 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Liming Gao, Yonghong Zhu, Eric Dong, Dandan Bi

The assignment operators for class ANTLRTokenPtr return void in current
code.

This commit makes them return the reference to the object just like
primitive types do.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h     | 4 ++--
 BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
index 75b4c86..df89d8f 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
+++ b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
@@ -58,8 +58,8 @@ public:
 //  7-Apr-97 133MR1
 //	     Fix suggested by Andreas Magnusson
 //			(Andreas.Magnusson@mailbox.swipnet.se)
-    void operator = (const ANTLRTokenPtr & lhs);		    	// MR1
-    void operator = (ANTLRAbstractToken *addr);
+    ANTLRTokenPtr& operator = (const ANTLRTokenPtr & lhs);      // MR1
+    ANTLRTokenPtr& operator = (ANTLRAbstractToken *addr);
     int operator != (const ANTLRTokenPtr &q) const	    		// MR1 // MR11 unsigned -> int
 	{ return this->ptr_ != q.ptr_; }
     int operator == (const ANTLRTokenPtr &q) const  			// MR1 // MR11 unsigned -> int
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
index 9c07cf5..a1efc3b 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
+++ b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
@@ -71,18 +71,20 @@ ANTLRTokenPtr::~ANTLRTokenPtr()
 //  8-Apr-97	MR1	Make operator -> a const member function
 //			  as weall as some other member functions
 //
-void ANTLRTokenPtr::operator = (const ANTLRTokenPtr & lhs)	// MR1
+ANTLRTokenPtr& ANTLRTokenPtr::operator = (const ANTLRTokenPtr & lhs)    // MR1
 {
     lhs.ref();	// protect against "xp = xp"; ie same underlying object
     deref();
     ptr_ = lhs.ptr_;
+    return *this;
 }
 
-void ANTLRTokenPtr::operator = (ANTLRAbstractToken *addr)
+ANTLRTokenPtr& ANTLRTokenPtr::operator = (ANTLRAbstractToken *addr)
 {
     if (addr != NULL) {
 	addr->ref();
     }
     deref();
     ptr_ = addr;
+    return *this;
 }
-- 
1.9.5.msysgit.0



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

* Re: [PATCH 00/52] Resolve issues for C source codes in BaseTools
  2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
                   ` (51 preceding siblings ...)
  2016-10-12 12:20 ` [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void Hao Wu
@ 2016-10-17  7:45 ` Gao, Liming
  52 siblings, 0 replies; 55+ messages in thread
From: Gao, Liming @ 2016-10-17  7:45 UTC (permalink / raw)
  To: Wu, Hao A, edk2-devel@lists.01.org; +Cc: Zhu, Yonghong, Dong, Eric, Bi, Dandan

Hao, 
  I have some comments for three patches. Others are good to me. 

Patch: BaseTools/TianoCompress: Avoid possible NULL pointer dereference
Comment:  Please free allocated buffer after error happens. 

Patch: BaseTools/C/Common: Fix parameter format mismatch in scanf functions
Comment:  Please add more description to say why INT32 is used in sscanf.

Patch: BaseTools/VolInfo: Use hard-coded format string for calls to sprintf()
Comment: Why the format string may be changed accidentally?

Thanks
Liming
> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, October 12, 2016 8:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Zhu, Yonghong <yonghong.zhu@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Bi, Dandan <dandan.bi@intel.com>
> Subject: [PATCH 00/52] Resolve issues for C source codes in BaseTools
> 
> The patch series fixes the following types of issues for C source codes in
> BaseTools:
> 
> 1. Avoid possible NULL pointer dereference
> 2. Initialize local variables before use
> 3. Remove unused local variables
> 4. Avoid accessing over array bounds
> 5. Resolve possible memory leak
> 6. Resolve file handles not being closed
> 7. Resolve possible buffer overflow in printf/scanf functions
> 
> The patch series is also available at:
> https://github.com/hwu25/edk2/tree/BaseTools_V1
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> 
> Hao Wu (52):
>   BaseTools/C/Common: Avoid possible NULL pointer dereference
>   BaseTools/EfiRom: Avoid possible NULL pointer dereference
>   BaseTools/GenFfs: Avoid possible NULL pointer dereference
>   BaseTools/GenFv: Avoid possible NULL pointer dereference
>   BaseTools/GenFw: Avoid possible NULL pointer dereference
>   BaseTools/GenPage: Avoid possible NULL pointer dereference
>   BaseTools/GenSec: Avoid possible NULL pointer dereference
>   BaseTools/GenVtf: Avoid possible NULL pointer dereference
>   BaseTools/TianoCompress: Avoid possible NULL pointer dereference
>   BaseTools/VfrCompile: Avoid possible NULL pointer dereference
>   BaseTools/VolInfo: Avoid possible NULL pointer dereference
>   BaseTools/TianoCompress: Initialize local variables before being used
>   BaseTools/VfrCompile: Initialize local variables before being used
>   BaseTools/GenBootSector: Fix parameter format mismatch in printf
>     functions
>   BaseTools/VolInfo: Fix parameter format mismatch in printf functions
>   BaseTools/C/Common: Fix parameter format mismatch in scanf functions
>   BaseTools/GenFv: Fix parameter format mismatch in scanf functions
>   BaseTools/GenFw: Fix parameter format mismatch in scanf functions
>   BaseTools/GenVtf: Fix parameter format mismatch in scanf functions
>   BaseTools/C/Common: Fix potential access over array bounds
>   BaseTools/EfiRom: Fix potential access over array bounds
>   BaseTools/GenFv: Fix potential access over array bounds
>   BaseTools/TianoCompress: Fix potential access over array bounds
>   BaseTools/VfrCompile: Fix potential access over array bounds
>   BaseTools/VfrCompile: Avoid freeing memory with mismatched functions
>   BaseTools/VfrCompile: Add assignment operator definition for some
>     classes
>   BaseTools/VfrCompile: Avoid freeing freed memory in classes
>   BaseTools/VfrCompile: Remove unused local variables
>   BaseTools/C/Common: Fix potential memory leak
>   BaseTools/EfiRom: Fix potential memory leak
>   BaseTools/GenFv: Fix potential memory leak
>   BaseTools/GenPage: Fix potential memory leak
>   BaseTools/GenSec: Fix potential memory leak
>   BaseTools/GenVtf: Fix potential memory leak
>   BaseTools/Split: Fix potential memory and resource leak
>   BaseTools/TianoCompress: Fix potential memory leak
>   BaseTools/VfrCompile: Fix potential memory leak
>   BaseTools/VolInfo: Fix potential memory leak
>   BaseTools/EfiRom: Fix file handles not being closed
>   BaseTools/GenBootSector: Fix file handles not being closed
>   BaseTools/GenCrc32: Fix file handles not being closed
>   BaseTools/GenFv: Fix file handles not being closed
>   BaseTools/GenVtf: Fix file handles not being closed
>   BaseTools/LzmaCompress: Fix file handles not being closed
>   BaseTools/TianoCompress: Fix file handles not being closed
>   BaseTools/VolInfo: Fix file handles not being closed
>   BaseTools/GenVtf: Fix potential buffer overflow in scanf functions
>   BaseTools/VolInfo: Fix potential buffer overflow in scanf functions
>   BaseTools/VfrCompile: Explicitly state format string for DebugMsg()
>   BaseTools/VolInfo: Use hard-coded format string for calls to sprintf()
>   BaseTools/VfrCompile/Pccts: Add virtual destructor for class
>     DLGInputStream
>   BaseTools/VfrCompile/Pccts: Make assignment operator not returning
>     void
> 
>  BaseTools/Source/C/Common/BasePeCoff.c             |  12 ++
>  BaseTools/Source/C/Common/CommonLib.c              |   8 +-
>  BaseTools/Source/C/Common/Decompress.c             |  41 ++++--
>  BaseTools/Source/C/Common/EfiUtilityMsgs.c         |  20 +--
>  BaseTools/Source/C/Common/FirmwareVolumeBuffer.c   |   6 +-
>  BaseTools/Source/C/Common/MemoryFile.c             |   3 +-
>  BaseTools/Source/C/Common/MyAlloc.c                |  55 +++++++-
>  .../Source/C/Common/ParseGuidedSectionTools.c      |  21 ++--
>  BaseTools/Source/C/Common/ParseInf.c               |  24 ++--
>  BaseTools/Source/C/Common/SimpleFileParsing.c      |  14 +--
>  BaseTools/Source/C/Common/TianoCompress.c          |   9 +-
>  BaseTools/Source/C/EfiRom/EfiRom.c                 | 120 ++++++++++++------
>  BaseTools/Source/C/GenBootSector/GenBootSector.c   |  43 ++++---
>  BaseTools/Source/C/GenCrc32/GenCrc32.c             |   3 +-
>  BaseTools/Source/C/GenFfs/GenFfs.c                 |  36 +++---
>  BaseTools/Source/C/GenFv/GenFv.c                   |   9 +-
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c        |  83 ++++++++++--
>  BaseTools/Source/C/GenFw/Elf32Convert.c            |   8 ++
>  BaseTools/Source/C/GenFw/Elf64Convert.c            |  10 +-
>  BaseTools/Source/C/GenFw/ElfConvert.c              |   7 +-
>  BaseTools/Source/C/GenFw/GenFw.c                   |  20 ++-
>  BaseTools/Source/C/GenPage/GenPage.c               |  12 +-
>  BaseTools/Source/C/GenSec/GenSec.c                 |  27 +++-
>  BaseTools/Source/C/GenVtf/GenVtf.c                 | 117 ++++++++++++++++-
>  BaseTools/Source/C/LzmaCompress/LzmaCompress.c     |   6 +-
>  BaseTools/Source/C/Split/Split.c                   |  41 ++++--
>  BaseTools/Source/C/TianoCompress/TianoCompress.c   |  68 ++++++----
>  BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h    |   4 +-
>  .../Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h      |   6 +-
>  BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h     |   3 +
>  BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h |   4 +
>  BaseTools/Source/C/VfrCompile/VfrCompiler.cpp      | 140
> ++++++++++++++++++---
>  BaseTools/Source/C/VfrCompile/VfrCompiler.h        |   8 +-
>  BaseTools/Source/C/VfrCompile/VfrError.cpp         |   4 +-
>  BaseTools/Source/C/VfrCompile/VfrError.h           |  10 +-
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp       |  29 +++--
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.h         |  13 ++
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp    |  54 +++++---
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.h      |  60 ++++++++-
>  BaseTools/Source/C/VolInfo/VolInfo.c               | 107 +++++++++++++---
>  40 files changed, 1004 insertions(+), 261 deletions(-)
> 
> --
> 1.9.5.msysgit.0



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

* Re: [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void
  2016-10-12 12:20 ` [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void Hao Wu
@ 2016-10-18  1:12   ` Dong, Eric
  0 siblings, 0 replies; 55+ messages in thread
From: Dong, Eric @ 2016-10-18  1:12 UTC (permalink / raw)
  To: Wu, Hao A, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong, Bi, Dandan

 Hi Hao,

I have below comments:

Case 1: In order to keep consistence, can you also change other cases which still initialized with '\0'

  mOptions.VfrFileName[0]                = '\0';
  mOptions.RecordListFile                = NULL;
  mOptions.CreateRecordListFile          = FALSE;
  mOptions.CreateIfrPkgFile              = FALSE;
  mOptions.PkgOutputFileName             = NULL;
  mOptions.COutputFileName               = NULL;
  mOptions.OutputDirectory[0]            = '\0';
  mOptions.PreprocessorOutputFileName    = NULL;
  mOptions.VfrBaseFileName[0]            = '\0';


Case 2: for below case, you can use the actual size for the strncpy instead of MAX_PATH - 1. Also the last line code is not needed.

      if (strlen (Argv[Index]) > MAX_PATH - 1) {
        DebugError (NULL, 0, 1003, "Invalid option value", "Output directory name %s is too long", Argv[Index]);
        goto Fail;
      }
      strncpy (mOptions.OutputDirectory, Argv[Index], MAX_PATH - 1);
      mOptions.OutputDirectory[MAX_PATH - 1] = 0;

Thanks,
Eric
> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, October 12, 2016 8:21 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; Gao, Liming; Zhu, Yonghong; Dong, Eric; Bi, Dandan
> Subject: [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void
> 
> The assignment operators for class ANTLRTokenPtr return void in current
> code.
> 
> This commit makes them return the reference to the object just like
> primitive types do.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h     | 4 ++--
>  BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h | 6 ++++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
> index 75b4c86..df89d8f 100644
> --- a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
> +++ b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h
> @@ -58,8 +58,8 @@ public:
>  //  7-Apr-97 133MR1
>  //	     Fix suggested by Andreas Magnusson
>  //			(Andreas.Magnusson@mailbox.swipnet.se)
> -    void operator = (const ANTLRTokenPtr & lhs);		    	// MR1
> -    void operator = (ANTLRAbstractToken *addr);
> +    ANTLRTokenPtr& operator = (const ANTLRTokenPtr & lhs);      // MR1
> +    ANTLRTokenPtr& operator = (ANTLRAbstractToken *addr);
>      int operator != (const ANTLRTokenPtr &q) const	    		// MR1 // MR11 unsigned -> int
>  	{ return this->ptr_ != q.ptr_; }
>      int operator == (const ANTLRTokenPtr &q) const  			// MR1 // MR11 unsigned -> int
> diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
> index 9c07cf5..a1efc3b 100644
> --- a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
> +++ b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h
> @@ -71,18 +71,20 @@ ANTLRTokenPtr::~ANTLRTokenPtr()
>  //  8-Apr-97	MR1	Make operator -> a const member function
>  //			  as weall as some other member functions
>  //
> -void ANTLRTokenPtr::operator = (const ANTLRTokenPtr & lhs)	// MR1
> +ANTLRTokenPtr& ANTLRTokenPtr::operator = (const ANTLRTokenPtr & lhs)    // MR1
>  {
>      lhs.ref();	// protect against "xp = xp"; ie same underlying object
>      deref();
>      ptr_ = lhs.ptr_;
> +    return *this;
>  }
> 
> -void ANTLRTokenPtr::operator = (ANTLRAbstractToken *addr)
> +ANTLRTokenPtr& ANTLRTokenPtr::operator = (ANTLRAbstractToken *addr)
>  {
>      if (addr != NULL) {
>  	addr->ref();
>      }
>      deref();
>      ptr_ = addr;
> +    return *this;
>  }
> --
> 1.9.5.msysgit.0



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

end of thread, other threads:[~2016-10-18  1:12 UTC | newest]

Thread overview: 55+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-12 12:19 [PATCH 00/52] Resolve issues for C source codes in BaseTools Hao Wu
2016-10-12 12:19 ` [PATCH 01/52] BaseTools/C/Common: Avoid possible NULL pointer dereference Hao Wu
2016-10-12 12:19 ` [PATCH 02/52] BaseTools/EfiRom: " Hao Wu
2016-10-12 12:19 ` [PATCH 03/52] BaseTools/GenFfs: " Hao Wu
2016-10-12 12:19 ` [PATCH 04/52] BaseTools/GenFv: " Hao Wu
2016-10-12 12:19 ` [PATCH 05/52] BaseTools/GenFw: " Hao Wu
2016-10-12 12:19 ` [PATCH 06/52] BaseTools/GenPage: " Hao Wu
2016-10-12 12:19 ` [PATCH 07/52] BaseTools/GenSec: " Hao Wu
2016-10-12 12:19 ` [PATCH 08/52] BaseTools/GenVtf: " Hao Wu
2016-10-12 12:19 ` [PATCH 09/52] BaseTools/TianoCompress: " Hao Wu
2016-10-12 12:19 ` [PATCH 10/52] BaseTools/VfrCompile: " Hao Wu
2016-10-12 12:19 ` [PATCH 11/52] BaseTools/VolInfo: " Hao Wu
2016-10-12 12:19 ` [PATCH 12/52] BaseTools/TianoCompress: Initialize local variables before being used Hao Wu
2016-10-12 12:19 ` [PATCH 13/52] BaseTools/VfrCompile: " Hao Wu
2016-10-12 12:19 ` [PATCH 14/52] BaseTools/GenBootSector: Fix parameter format mismatch in printf functions Hao Wu
2016-10-12 12:19 ` [PATCH 15/52] BaseTools/VolInfo: " Hao Wu
2016-10-12 12:20 ` [PATCH 16/52] BaseTools/C/Common: Fix parameter format mismatch in scanf functions Hao Wu
2016-10-12 12:20 ` [PATCH 17/52] BaseTools/GenFv: " Hao Wu
2016-10-12 12:20 ` [PATCH 18/52] BaseTools/GenFw: " Hao Wu
2016-10-12 12:20 ` [PATCH 19/52] BaseTools/GenVtf: " Hao Wu
2016-10-12 12:20 ` [PATCH 20/52] BaseTools/C/Common: Fix potential access over array bounds Hao Wu
2016-10-12 12:20 ` [PATCH 21/52] BaseTools/EfiRom: " Hao Wu
2016-10-12 12:20 ` [PATCH 22/52] BaseTools/GenFv: " Hao Wu
2016-10-12 12:20 ` [PATCH 23/52] BaseTools/TianoCompress: " Hao Wu
2016-10-12 12:20 ` [PATCH 24/52] BaseTools/VfrCompile: " Hao Wu
2016-10-12 12:20 ` [PATCH 25/52] BaseTools/VfrCompile: Avoid freeing memory with mismatched functions Hao Wu
2016-10-12 12:20 ` [PATCH 26/52] BaseTools/VfrCompile: Add assignment operator definition for some classes Hao Wu
2016-10-12 12:20 ` [PATCH 27/52] BaseTools/VfrCompile: Avoid freeing freed memory in classes Hao Wu
2016-10-12 12:20 ` [PATCH 28/52] BaseTools/VfrCompile: Remove unused local variables Hao Wu
2016-10-12 12:20 ` [PATCH 29/52] BaseTools/C/Common: Fix potential memory leak Hao Wu
2016-10-12 12:20 ` [PATCH 30/52] BaseTools/EfiRom: " Hao Wu
2016-10-12 12:20 ` [PATCH 31/52] BaseTools/GenFv: " Hao Wu
2016-10-12 12:20 ` [PATCH 32/52] BaseTools/GenPage: " Hao Wu
2016-10-12 12:20 ` [PATCH 33/52] BaseTools/GenSec: " Hao Wu
2016-10-12 12:20 ` [PATCH 34/52] BaseTools/GenVtf: " Hao Wu
2016-10-12 12:20 ` [PATCH 35/52] BaseTools/Split: Fix potential memory and resource leak Hao Wu
2016-10-12 12:20 ` [PATCH 36/52] BaseTools/TianoCompress: Fix potential memory leak Hao Wu
2016-10-12 12:20 ` [PATCH 37/52] BaseTools/VfrCompile: " Hao Wu
2016-10-12 12:20 ` [PATCH 38/52] BaseTools/VolInfo: " Hao Wu
2016-10-12 12:20 ` [PATCH 39/52] BaseTools/EfiRom: Fix file handles not being closed Hao Wu
2016-10-12 12:20 ` [PATCH 40/52] BaseTools/GenBootSector: " Hao Wu
2016-10-12 12:20 ` [PATCH 41/52] BaseTools/GenCrc32: " Hao Wu
2016-10-12 12:20 ` [PATCH 42/52] BaseTools/GenFv: " Hao Wu
2016-10-12 12:20 ` [PATCH 43/52] BaseTools/GenVtf: " Hao Wu
2016-10-12 12:20 ` [PATCH 44/52] BaseTools/LzmaCompress: " Hao Wu
2016-10-12 12:20 ` [PATCH 45/52] BaseTools/TianoCompress: " Hao Wu
2016-10-12 12:20 ` [PATCH 46/52] BaseTools/VolInfo: " Hao Wu
2016-10-12 12:20 ` [PATCH 47/52] BaseTools/GenVtf: Fix potential buffer overflow in scanf functions Hao Wu
2016-10-12 12:20 ` [PATCH 48/52] BaseTools/VolInfo: " Hao Wu
2016-10-12 12:20 ` [PATCH 49/52] BaseTools/VfrCompile: Explicitly state format string for DebugMsg() Hao Wu
2016-10-12 12:20 ` [PATCH 50/52] BaseTools/VolInfo: Use hard-coded format string for calls to sprintf() Hao Wu
2016-10-12 12:20 ` [PATCH 51/52] BaseTools/VfrCompile/Pccts: Add virtual destructor for class DLGInputStream Hao Wu
2016-10-12 12:20 ` [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void Hao Wu
2016-10-18  1:12   ` Dong, Eric
2016-10-17  7:45 ` [PATCH 00/52] Resolve issues for C source codes in BaseTools Gao, Liming

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