From: "Gao, Liming" <liming.gao@intel.com>
To: "Wu, Hao A" <hao.a.wu@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Zhu, Yonghong" <yonghong.zhu@intel.com>,
"Dong, Eric" <eric.dong@intel.com>,
"Bi, Dandan" <dandan.bi@intel.com>
Subject: Re: [PATCH 00/52] Resolve issues for C source codes in BaseTools
Date: Mon, 17 Oct 2016 07:45:57 +0000 [thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14B49531E@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1476274836-10544-1-git-send-email-hao.a.wu@intel.com>
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
prev parent reply other threads:[~2016-10-17 7:46 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Gao, Liming [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A89E2EF3DFEDB4C8BFDE51014F606A14B49531E@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox