From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.93; helo=mga11.intel.com; envelope-from=chao.b.zhang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (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 1E15F2063D77A for ; Mon, 16 Jul 2018 08:10:39 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Jul 2018 08:10:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,361,1526367600"; d="scan'208";a="216416007" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga004.jf.intel.com with ESMTP; 16 Jul 2018 08:09:45 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 16 Jul 2018 08:09:44 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.57]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.173]) with mapi id 14.03.0319.002; Mon, 16 Jul 2018 23:09:43 +0800 From: "Zhang, Chao B" To: "rbacik@gmail.com" , "edk2-devel@lists.01.org" CC: "Yao, Jiewen" , Laszlo Ersek , Vladimir Olovyannikov Thread-Topic: [PATCH v2] SecurityPkg: Fix assert when setting key from eMMC/SD/USB Thread-Index: AQHUGKCBK3DE1zNIlEK6Yry5P4oiOqSRXb3Q Date: Mon, 16 Jul 2018 15:09:42 +0000 Message-ID: References: <20180710225105.28443-1-roman.bacik@broadcom.com> In-Reply-To: <20180710225105.28443-1-roman.bacik@broadcom.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTQzMDcyMmQtNjJhYi00YmM5LWIyMjMtYTZjZTFlNDEzMDdhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibFdRZzJ2UkxiM08yWlNNRURpaTlHVTVLekwwM1pSTGNVVko0T1hmUTZyUWNOQk54UUxDRFdMMWRJVllRUXRhSSJ9 dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2] SecurityPkg: Fix assert when setting key from eMMC/SD/USB X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Jul 2018 15:10:39 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Bacik: Tks for the fix. Would you please file another report in Bugzilla for Ra= mDisk & Tls Configuration driver? They have same issue as SecureBootConfig = driver -----Original Message----- From: rbacik@gmail.com [mailto:rbacik@gmail.com]=20 Sent: Wednesday, July 11, 2018 6:51 AM To: edk2-devel@lists.01.org Cc: Zhang, Chao B ; Yao, Jiewen ; Laszlo Ersek ; Vladimir Olovyannikov Subject: [PATCH v2] SecurityPkg: Fix assert when setting key from eMMC/SD/U= SB From: Roman Bacik When secure boot is enabled, if one loads keys from a FAT formatted eMMC/SD= /USB when trying to provision PK/KEK/DB keys via the menu, an assert in Str= Len() occurs. This is because the filename starts on odd address, which is not a uint16 a= ligned boundary: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1003 Cc: Chao Zhang Cc: Jiewen Yao Cc: Laszlo Ersek Cc: Vladimir Olovyannikov Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Roman Bacik --- SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFile= Explorer.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBo= otConfigFileExplorer.c b/SecurityPkg/VariableAuthenticated/SecureBootConfig= Dxe/SecureBootConfigFileExplorer.c index 1b6f88804275..19b13a5569a6 100644 --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi= gFileExplorer.c +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo +++ nfigFileExplorer.c @@ -123,6 +123,8 @@ OpenFileByDevicePath( EFI_FILE_PROTOCOL *Handle1; EFI_FILE_PROTOCOL *Handle2; EFI_HANDLE DeviceHandle; + CHAR16 *PathName; + UINTN PathLength; =20 if ((FilePath =3D=3D NULL || FileHandle =3D=3D NULL)) { return EFI_INVALID_PARAMETER; @@ -173,6 +175,11 @@ OpenFileByDevicePath( // Handle2 =3D Handle1; Handle1 =3D NULL; + PathLength =3D DevicePathNodeLength(*FilePath) - sizeof(EFI_DEVICE_PAT= H_PROTOCOL); + PathName =3D AllocateCopyPool(PathLength, ((FILEPATH_DEVICE_PATH*)*Fil= ePath)->PathName); + if (PathName =3D=3D NULL) { + return EFI_OUT_OF_RESOURCES; + } =20 // // Try to test opening an existing file @@ -180,7 +187,7 @@ OpenFileBy= DevicePath( Status =3D Handle2->Open ( Handle2, &Handle1, - ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName, + PathName, OpenMode &~EFI_FILE_MODE_CREATE, 0 ); @@ -192,7 +199,7 @@ OpenFileByDevicePath( Status =3D Handle2->Open ( Handle2, &Handle1, - ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName, + PathName, OpenMode, Attributes ); @@ -202,6 +209,8 @@ OpenFileByDevicePath( // Handle2->Close (Handle2); =20 + FreePool (PathName); + if (EFI_ERROR(Status)) { return (Status); } -- 2.17.1