From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 5DE9681F27 for ; Thu, 9 Feb 2017 07:27:50 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP; 09 Feb 2017 07:27:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,349,1484035200"; d="scan'208,217";a="42047623" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga002.jf.intel.com with ESMTP; 09 Feb 2017 07:27:43 -0800 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 9 Feb 2017 07:27:39 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 9 Feb 2017 07:27:38 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.177]) with mapi id 14.03.0248.002; Thu, 9 Feb 2017 23:27:36 +0800 From: "Yao, Jiewen" To: Ard Biesheuvel CC: "Tian, Feng" , "edk2-devel@lists.01.org" , Leif Lindholm , "Kinney, Michael D" , "Fan, Jeff" , "Zeng, Star" Thread-Topic: [edk2] [PATCH V3 0/4] DXE Memory Protection Thread-Index: AQHSgqUbN7Jz0f1B3Eq2SFYIOVpghqFgSkIg//+NEACAAAWbAIAAA7mAgADBeLD//4m9AIAAh6wg//+KMQAAESR5oA== Date: Thu, 9 Feb 2017 15:27:35 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A8EC023@shsmsx102.ccr.corp.intel.com> References: <1486624832-15736-1-git-send-email-jiewen.yao@intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8EBD52@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8EBEC3@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8EBF20@shsmsx102.ccr.corp.intel.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 Subject: Re: [PATCH V3 0/4] DXE Memory Protection 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: Thu, 09 Feb 2017 15:27:50 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 1) That is great. I appreciate your quick response and help. I will drop my patch for ARM 2/4, and wait for yours. 2) For ImageEnd alignment issue, I agree with you. I plan to round up with: ImageRecord->ImageSize =3D ALIGN_VALUE(LoadedImage->ImageSize, SectionAlign= ment); before SetUefiImageProtectionAttributes (ImageRecord, Protect); I will update it at V4. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard = Biesheuvel Sent: Thursday, February 9, 2017 6:56 AM To: Yao, Jiewen Cc: Tian, Feng ; edk2-devel@lists.01.org; Leif Lindhol= m ; Kinney, Michael D ; Fan, Jeff ; Zeng, Star Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection On 9 February 2017 at 14:08, Yao, Jiewen > wrote: > For X86 CPU, the memory protection attribute goes to page table, the cach= e > attribute goes to MTRR register. > > Those are 2 difference resource, and can be set separately. > > > > The high level pseudo code is below: > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > CacheAttribute =3D Attribute & CACHE_ATTRIBUTE_MASK > > MemoryProtectionAttribute =3D Attribute & MEMORY_PROTECTION_ATTRIBUTE_MAS= K > > If (CacheAttribute !=3D 0) { > > SetCacheAttribute(CacheAttribute) > > } > > SetMemoryProtectionAttribute(MemoryProtectionAttribute) > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > NOTE: we need compare CacheAttribute =3D=3D 0, because the cache attribut= e is an > individual mask. 0x1 means UN_CACHE, 0x8 means WRITE_BACK. 0 is meaningle= ss. > > We do not compare MemoryProtectionAttribute =3D=3D 0, because 0 is a vali= d > memory protection attribute, which means to disable protection. > > > > Before GCD sync, the DxeCore does not know the cache attribute, so that i= t > can only set memory attribute. The CPU driver only modifies page table ba= sed > upon MemoryProtectionAttribute and keep cache attribute untouched. > OK, I think we should be able to work around this, although it is not pretty. I will send it out as a separate patch. I do have one other question though: would it be possible to round up the end of the image to page size in this code? (in SetUefiImageProtectionAttributes()) Otherwise, we may end up calling SetMemoryAttributes() with a length that is not page aligned, which hits an EFI_UNSUPPORTED on ARM. Given that the PE/COFF loader always allocates full pages, we know the space after the image is not used, so it is possible (and even more secure!) to clear the exec bit on it. """ // // Last DATA // ASSERT (CurrentBase <=3D ImageEnd); if (CurrentBase < ImageEnd) { // // DATA // if (Protect) { Attribute =3D EFI_MEMORY_XP; } else { Attribute =3D 0; } SetUefiImageMemoryAttributes ( CurrentBase, ImageEnd - CurrentBase, Attribute ); } """ _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel