From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (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 4D682203B8589 for ; Wed, 9 May 2018 01:22:31 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 May 2018 01:22:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,381,1520924400"; d="scan'208";a="57153051" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga002.jf.intel.com with ESMTP; 09 May 2018 01:22:31 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 9 May 2018 01:22:31 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 9 May 2018 01:22:30 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.40]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.179]) with mapi id 14.03.0319.002; Wed, 9 May 2018 16:22:28 +0800 From: "Dong, Eric" To: "Bi, Dandan" , "edk2-devel@lists.01.org" CC: "Gao, Liming" , Gary Lin Thread-Topic: [patch v2] BaseTools/VfrCompile: Avoid using uninitialized pointer Thread-Index: AQHT51Lybze+lPKJ9UKUJnjgzOnuRqQnDwOw Date: Wed, 9 May 2018 08:22:27 +0000 Message-ID: References: <20180509050211.76104-1-dandan.bi@intel.com> In-Reply-To: <20180509050211.76104-1-dandan.bi@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 v2] BaseTools/VfrCompile: Avoid using uninitialized pointer X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 May 2018 08:22:32 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Eric Dong -----Original Message----- From: Bi, Dandan=20 Sent: Wednesday, May 9, 2018 1:02 PM To: edk2-devel@lists.01.org Cc: Dong, Eric ; Gao, Liming ; G= ary Lin Subject: [patch v2] BaseTools/VfrCompile: Avoid using uninitialized pointer V2: Add function _INIT_OPHDR_COND () for variable initialization. Make code logic more clean. Previously _CLEAR_SAVED_OPHDR () is used for variable initialization, and w= e updated it to clean memory. But _CLEAR_SAVED_OPHDR () is still called for variable initialization. This= will cause uninitialized pointer will be checked to free and cause unexpec= ted issue. This patch is to add new function for variable initialization and keep _CLE= AR_SAVED_OPHDR () to clean memory which is aligned with its function name. Cc: Eric Dong Cc: Liming Gao Cc: Gary Lin Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Dandan Bi --- BaseTools/Source/C/VfrCompile/VfrSyntax.g | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g b/BaseTools/Source/C= /VfrCompile/VfrSyntax.g index 4b0a43606ea..84dd2c3ed3f 100644 --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g @@ -4084,11 +4084,19 @@ vfrStatementInvalidSaveRestoreDefaults : =20 // // Root expression extension function called by other function. // vfrStatementExpression [UINT32 RootLevel, UINT32 ExpOpCount =3D 0] : - << if ($RootLevel =3D=3D 0) {mCIfrOpHdrIndex ++; if (mCIfrOpHdrIndex >= =3D MAX_IFR_EXPRESSION_DEPTH) _PCATCH (VFR_RETURN_INVALID_PARAMETER, 0, "Th= e depth of expression exceeds the max supported level 8!"); _CLEAR_SAVED_OP= HDR ();} >> + << + if ($RootLevel = =3D=3D 0) { + mCIfrOpHdrInde= x ++; + if (mCIfrOpHdr= Index >=3D MAX_IFR_EXPRESSION_DEPTH) { + _PCATCH (VFR= _RETURN_INVALID_PARAMETER, 0, "The depth of expression exceeds the max supp= orted level 8!"); + } + _INIT_OPHDR_CO= ND (); + } + >> andTerm[$RootLevel, $ExpOpCount] ( L:OR andTerm[$RootLevel, $ExpOpCount] << $ExpOpCount++; C= IfrOr OObj(L->getLine()); >> )* << @@ -4988,10 +499= 6,11 @@ private: CIfrOpHeader * mCIfrOpHdr[MAX_IFR_EXPRESSION_DEPTH]; UINT32 mCIfrOpHdrLineNo[MAX_IFR_EXPRESSION_DEPTH]; UINT8 mCIfrOpHdrIndex; VOID _SAVE_OPHDR_COND (IN CIfrOpHeader &, IN BOOLEAN, UIN= T32 LineNo =3D 0); VOID _CLEAR_SAVED_OPHDR (VOID); + VOID _INIT_OPHDR_COND (VOID); BOOLEAN _SET_SAVED_OPHDR_SCOPE (VOID); =20 =20 EFI_VARSTORE_INFO mCurrQestVarInfo; EFI_GUID *mOverrideClassGuid; @@ -5077,20 +5086,28 @@ EfiVfrParser::_SAVE_OPHDR_COND ( mCIfrOpHdr[mCIfrOpHdrIndex] =3D new CIfrOpHeader(OpHdr); mCIfrOpHdrLineNo[mCIfrOpHdrIndex] =3D LineNo; } } =20 +VOID +EfiVfrParser::_INIT_OPHDR_COND ( + VOID + ) +{ + mCIfrOpHdr[mCIfrOpHdrIndex] =3D NULL; + mCIfrOpHdrLineNo[mCIfrOpHdrIndex] =3D 0; } + VOID EfiVfrParser::_CLEAR_SAVED_OPHDR ( VOID ) { if (mCIfrOpHdr[mCIfrOpHdrIndex] !=3D NULL) { delete mCIfrOpHdr[mCIfrOpHdrIndex]; - mCIfrOpHdr[mCIfrOpHdrIndex] =3D NULL; + mCIfrOpHdr[mCIfrOpHdrIndex] =3D NULL; } - mCIfrOpHdrLineNo[mCIfrOpHdrIndex] =3D 0; } =20 BOOLEAN EfiVfrParser::_SET_SAVED_OPHDR_SCOPE ( VOID -- 2.14.3.windows.1