From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C787A1A1E20 for ; Mon, 17 Oct 2016 00:46:01 -0700 (PDT) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga105.fm.intel.com with ESMTP; 17 Oct 2016 00:46:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,356,1473145200"; d="scan'208";a="20273299" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga006.fm.intel.com with ESMTP; 17 Oct 2016 00:46:01 -0700 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 17 Oct 2016 00:46:01 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx120.amr.corp.intel.com (10.18.124.208) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 17 Oct 2016 00:46:00 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.206]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.139]) with mapi id 14.03.0248.002; Mon, 17 Oct 2016 15:45:58 +0800 From: "Gao, Liming" To: "Wu, Hao A" , "edk2-devel@lists.01.org" CC: "Zhu, Yonghong" , "Dong, Eric" , "Bi, Dandan" Thread-Topic: [PATCH 00/52] Resolve issues for C source codes in BaseTools Thread-Index: AQHSJIMfDL+61ZXeZES3Qe8H+8s65KCsSznw Date: Mon, 17 Oct 2016 07:45:57 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14B49531E@shsmsx102.ccr.corp.intel.com> References: <1476274836-10544-1-git-send-email-hao.a.wu@intel.com> In-Reply-To: <1476274836-10544-1-git-send-email-hao.a.wu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 00/52] Resolve issues for C source codes in BaseTools X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Oct 2016 07:46:02 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hao,=20 I have some comments for three patches. Others are good to me.=20 Patch: BaseTools/TianoCompress: Avoid possible NULL pointer dereference Comment: Please free allocated buffer after error happens.=20 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 ; Gao, Liming ; > Zhu, Yonghong ; Dong, Eric > ; Bi, Dandan > Subject: [PATCH 00/52] Resolve issues for C source codes in BaseTools >=20 > The patch series fixes the following types of issues for C source codes i= n > BaseTools: >=20 > 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 >=20 > The patch series is also available at: > https://github.com/hwu25/edk2/tree/BaseTools_V1 >=20 > Cc: Liming Gao > Cc: Yonghong Zhu > Cc: Eric Dong > Cc: Dandan Bi >=20 > 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 >=20 > 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(-) >=20 > -- > 1.9.5.msysgit.0