From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web10.7279.1675419056604553458 for ; Fri, 03 Feb 2023 02:10:57 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=dmLQvehw; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: jiewen.yao@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1675419056; x=1706955056; h=from:to:subject:date:message-id:references:in-reply-to: mime-version; bh=JVMjVxZLTSP38hKmC6ruFslD+9b64FaA0WYYyz+Jv5s=; b=dmLQvehwSOFCOq5meRumH+7OB2f3zlKcdMjBbW/KnlxMOwWv+oCh7Xg0 vORcgmPgKBktfTCsb2K0MQm3hYWd8uuMJbRGmPqyVL4Z5XTPSJOf3/hSy l/GLhCP7XleRoDRZXxooNAhbZu8xJ0XRyee4Wf0AJ1PPtU+agv7r1SzIm L1oNH9BCq5LEBvpAec5BJAWEeOzsPvtJr5+9rPspV/7yl8lChLKaVleK8 7LedMjOo4oqbVadKNa23WQrPVp7Dm0/+RFYNwyJFVukkh6uFvMrovlp4p rfq5qW8HWuMyV7tkWjnqRAi3DyS0wGNhDEkMOhLtwPTNoFtTa+Z5DQsEq Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10609"; a="309046082" X-IronPort-AV: E=Sophos;i="5.97,270,1669104000"; d="scan'208,217";a="309046082" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Feb 2023 02:10:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10609"; a="754426782" X-IronPort-AV: E=Sophos;i="5.97,270,1669104000"; d="scan'208,217";a="754426782" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by FMSMGA003.fm.intel.com with ESMTP; 03 Feb 2023 02:10:39 -0800 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Fri, 3 Feb 2023 02:10:38 -0800 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Fri, 3 Feb 2023 02:10:38 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Fri, 3 Feb 2023 02:10:38 -0800 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.173) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Fri, 3 Feb 2023 02:10:37 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mccHKLmUUMUR5eD6zmkm0l7TJ4/t+kSawwAlpQKnrS2d8EfDmARAfgpdbS9J5cLi3AppogHuhkO4QAQdRb5OY0DhPh4DyZ1vcAqPnJ44q5c6Nb2nqGey0ZJQ15AbULyLuS0svm6mCSma6eiQl647nlCEiKEFf/UTOdJIrbaSwXk7N4XE12mC+inXIFW5P/VJvsyMBu8jyD3VvPoQDOCX9NDRUltB3WoR7sGfiLMuVmCKPIhVDxw8FhsfHSwtSBWKTAFj+ZOcARcmgQmH6TK7OYuiNcT4Fe+nLu08xn/BwKKan3rLY7BGGRRjH5uJvRlPbDJIF2nGiLiHYnk186jpoQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=wtKvAp7SXk8LOfxNfOBvj3XbiXU/gXAFkeSXawCKblc=; b=nYaf0K2/PK/QoO/Os/AjyVWLSWozlGTYGvcOBMcWJBkVyhZnDyyyOevbdYvDmUabekn0PW9qoDBbQvzNBtngjdIVarTW652hsT8iC17JAYqRjj/jcxpYoKVdhq6W3BioS9YWLVujwyTsU02aiPPYizccoGYBpwpLUOYFrksGbzrvDe13vjHlFfXqJUPX4jZWBF5xVbIQXumYIMn1LHIi86ErkzQN3u86n6TuM85rljuVX1pq07va1dFUsHuiKcCkFd1os7QMEyKUr4kPQ5Cruerv5zkeY+bE1N4kexGcLi/Xb0v8yiZS3GtDTu9B/gvlc8kNlLToVZ5icSL1TXMgSA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from MW4PR11MB5872.namprd11.prod.outlook.com (2603:10b6:303:169::14) by MW5PR11MB5810.namprd11.prod.outlook.com (2603:10b6:303:192::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6064.27; Fri, 3 Feb 2023 10:10:30 +0000 Received: from MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::96f4:ad8:3fb9:b60d]) by MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::96f4:ad8:3fb9:b60d%3]) with mapi id 15.20.6064.023; Fri, 3 Feb 2023 10:10:30 +0000 From: "Yao, Jiewen" To: "devel@edk2.groups.io" , "mhaeuser@posteo.de" , Ard Biesheuvel Subject: Re: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in mem attributes table Thread-Topic: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in mem attributes table Thread-Index: AQHZNzEnDKZLE01tG0y1vGlgAljmPq67/6EAgAADUQCAAFpSAIAAC7KQgAB7C4CAABfygIAAAeDA Date: Fri, 3 Feb 2023 10:10:30 +0000 Message-ID: References: <17027.1675417923801217060@groups.io> In-Reply-To: <17027.1675417923801217060@groups.io> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MW4PR11MB5872:EE_|MW5PR11MB5810:EE_ x-ms-office365-filtering-correlation-id: 51f0cf3c-0bc3-4f1f-69a4-08db05cee14f x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: WKnz9hJqjrav0FGbybIg/mAHEKNTb+Lh8nyYoPvBjGjSVu6d+IpUn9N91cRDnlGAh5oVMknXPaPjWzhIDzFWhgMi7oaWvs7wT91623EZtE2lnKYAs3iWHqFI62IiZDZrjcNTvKLnaripmo7z03oE559dO7SBy/LE2td4kslHMQETFjHrswbK2QYhnrM4IxpPKvp2nCWMJZM1D+Xhx36yiwaDQlrALRelVZFLWqPeb8jd5ujO7GqJFdrmigNIxHRUdLxjgfsiuhACWdqQzjlRYXE2IB2pQPy/o/qgITfweMpeLwpadOfhLx8Ml6mtjll9/Lle1bazDc68jW3h4pODA96zXPY1ysv4THvu+dMmz9X34floqXq8aTEHmNb61yoRB+rxsDT2vFdqaGE2mPv2OEVBFIPFmIm30HkG9KqiG2+QJ5PFtliphUoK3gmlFiszaDFCHI5oM/8e1zTEdp7J+5lNiry9k51wGCcKerFqdnPgGMwEDm8BSUb5Fo/OBKreiEl5/o7l1m9AB2Fb2KFovdat32tluavD7Wrm9gbndM0Pvb+/DRnj5XyyXVqjMXOfEIalds3KAwDYzPeoT2xtXcTOoM0dVfyETllA9joa4GzGeZTzvgFHn7cKA0vGWFXK8lJO6pEp6U6a+pYekxHWR92XhLrPNf3xcgzvChzERtvWGOBN5QJripKrNElZZzirw+qyfRV0xe9dSzgVGIHhOKFpNVExsFXk5j28VV4mMIGXtA40rPk/plQfwr+rBxJaB9nLOBRijfnJ7SJExDSRkQ== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW4PR11MB5872.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230025)(136003)(346002)(366004)(376002)(39860400002)(396003)(451199018)(66946007)(76116006)(8676002)(7696005)(478600001)(6506007)(26005)(110136005)(53546011)(33656002)(9686003)(45080400002)(316002)(186003)(76236004)(5660300002)(86362001)(2906002)(52536014)(8936002)(30864003)(66556008)(41300700001)(64756008)(66476007)(71200400001)(66446008)(55016003)(66574015)(83380400001)(38070700005)(122000001)(38100700002)(66899018)(166002)(82960400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?VnNNU+lb8nzq/mZ/Z9ukAFgUHgoUA3xNxSttvnQn0fNlcQMZTnnYeCtwWq?= =?iso-8859-1?Q?+XiITn3qJ/3imCV0ZZYC9JB/Bg0vNSO7dAd7l4k1R7MAwCRlvnMNb/AAH3?= =?iso-8859-1?Q?pi/mMCrqrvumFfa9IjS51BlE2QgsbQYrZAr5XahPkmXT2x3eJ9jycfIrVC?= =?iso-8859-1?Q?OMziZcO5f2WN4pJqTxaZaxr/9FZ0Yw0mfB1TAubONxwvcj1idWwoOuqIb6?= =?iso-8859-1?Q?/tUdMf30UFBqZj6yT1fL7mYfiHARmzqwtq+sJ+9ZcKoAm7DRrY7Qij1cL2?= =?iso-8859-1?Q?1LmJwJx6aSX9iX2PWSpYyX7Iv6/+Y3xk89FuId3eIaqSQB+NzIXdDcoTYl?= =?iso-8859-1?Q?a3blU3mNDMGm9ymcfUDQtmmVS5lzql1mDBggPTWpF7efdqUvLNWMmsVsKr?= =?iso-8859-1?Q?dIrpkIp9Rmjzja4XWMY7MCy+Wri9+ib/JAPXCcgPx2w33Mvkz7Loie4oyo?= =?iso-8859-1?Q?VgzPJnzvpQLt+tyNsbkNWJoh5qpAVUzNRICWakqsM8HMZDeX/xYAKloizP?= =?iso-8859-1?Q?yL4hDzSXMHvOZ+WErmMagkjLdcP3px9tPCfRIYFbepQFy7TKMRl4tNQSfv?= =?iso-8859-1?Q?1MRuYcMbTVK0Tho5WDHvtnLuedSFRd9mY27fn8R8s10r0Po2+EXuygmded?= =?iso-8859-1?Q?v+zta9yCE75MUHb4ox5qjvPiH4njlXogU66hHuGNZflHUMPw3k6Uii/JqC?= =?iso-8859-1?Q?FAFcJS44QFEAJRTZ0EL807OB9nZjoX3qRPaQBgbVhETgRCVFkT7RDCgYu5?= =?iso-8859-1?Q?77EAnU2C3ZR8XOCYc2/JXDHzROIHmFfG3PfpLcVusdqJRi5vfyg2au3o7i?= =?iso-8859-1?Q?O9pBIp8klrU+JItWHtc5yOS/twLgrGg9i4anxI3nvBQZ3rqM8uzABq2YOg?= =?iso-8859-1?Q?wHmas4DtEu698lNRXplzeoCiYoCgc8JinMMYYiJcwT/zsu7RQ6y+6w55y7?= =?iso-8859-1?Q?ZFw/AwIp7aX9ZKxNe8xIOLw8cPqwHogWfpqtaMahausD40IasWLC3vXY9+?= =?iso-8859-1?Q?6JXh4ANeXteNOSWZ4MBYZPVcD8RQDX9Qy2kxUFa5J6z0uu+jBFXkYEqXHn?= =?iso-8859-1?Q?3tSeqynNuhbes6MKPu+G8okv4IRD8wgu8ozSK8DTDhutTCN6OVssiEK0nh?= =?iso-8859-1?Q?rVzQbCFlQ6AyOuGJbqj1YDd4CiSGtGyF3PA78561hy3+PgHRvhJA3WBCtX?= =?iso-8859-1?Q?w6TmgfsygSpw1bSL17wKwkh0qKAtRqDpYjnOknwlSuffIjGGOo1SzbYLG9?= =?iso-8859-1?Q?HWoa/1hEJVIUh3tHymFrhA9cxODPQmcTm9RJVuUjvDULlppokhYG1wXfWW?= =?iso-8859-1?Q?NIPcU29Gv54lhpqQ56jBSmBH7aHQrXAXkZ4CmmPUhh7TPIFTcIV99eePB6?= =?iso-8859-1?Q?chhg9Zexle8EvrE1mSzbHKTkYHlM7ZVkShK7DkylZoaxmn1ifvpkF71Qpd?= =?iso-8859-1?Q?SNu9+9sgm/2XruWs8IhrKPHAFbmMChF+ps/tw5IvCd4X//pF8TKRjZW3Ma?= =?iso-8859-1?Q?QtAlgs4+o8rEaHgBb2RfFmOEIhhash1wkqGfKqFMmXsdLnp//+NKyqKuzv?= =?iso-8859-1?Q?wYLiQ0FTOfelitkW316Or1vm0/PYhv98+VAmnPVVk0Ud0omujv0pPq065S?= =?iso-8859-1?Q?wmFEmkEfmA92ilc7DtsewNO6/vyqyDmSQc?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5872.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 51f0cf3c-0bc3-4f1f-69a4-08db05cee14f X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Feb 2023 10:10:30.4224 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: beOZbB8hhAtQ3Px3MbUDaF/YnX+CyiwOC7IuSUqQ5y1yFPBiQrh6Ed29XYOJBaKS5sKKMrYPWzQ3nsOe/fp85w== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR11MB5810 Return-Path: jiewen.yao@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MW4PR11MB58726E1FBE8D9206D8B9137C8CD79MW4PR11MB5872namp_" --_000_MW4PR11MB58726E1FBE8D9206D8B9137C8CD79MW4PR11MB5872namp_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Adding bit in Image header is best way. I totally agree. The only disadvantage is that it may take time to update PE/COFF specificat= ion and take time to update the compiler to generate such bit. If people want to wait for those spec update, I don't have any concern. Personally, I don't think adding bit in FFS is a good idea. An image may co= me from anywhere - Flash, PCI card, Disk, Network. Hard to scale. Thank you Yao, Jiewen From: devel@edk2.groups.io On Behalf Of Marvin H=E4u= ser Sent: Friday, February 3, 2023 5:52 PM To: Ard Biesheuvel ; devel@edk2.groups.io Subject: Re: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forward edge= CFI in mem attributes table Hi Ard and Jiewen, (I'm replying from groups.io and cannot figure out how to CC Jiewen. Ugh.) Personally, I'd rather have UEFI itself rely solely on the flag in the imag= e file. If there is a way needed to handle images without the tag, in my op= inion use some userland preprocessing tool to check and annotate it when ge= nerating the firmware image. Even if it's trivial for Intel, I hope we won'= t end up with a micro-disassembler in firmware just to tell whether a featu= re is enabled. Best regards, Marvin On Fri, Feb 3, 2023 at 12:26 AM, Ard Biesheuvel wrote: (cc Samer, Jose) On Fri, 3 Feb 2023 at 02:16, Yao, Jiewen > wrote: Hello Can we assume that the entrypoint of PE/COFF image is always ENDBR64, if th= e PE/COFF image is enlightened to support IBT? I believe the compiler should do that, because the loader need use indirect= call to the PE/COFF entrypoint. That is an interesting idea. Even if we have some PE level annotation for BTI/IBT at some point, it would be a good sanity check on the image, because if the entry point does not have the guard instruction, there is obviously something wrong. However, on arm64, it is a bit more ambiguous, because there are instructions for pointer authentication (PACIASP) that may be interpreted as implicit guard instructions too, so the presence of such an instruction at the entry point does not imply that BTI is enabled in the entire image. I'll code up a PoC with this, but us ARM folks should have a think about how to spec this out and deploy this. I wonder In the mean time, Michael is trying to round up the MS folks that are involved in maintaining the PE/COFF spec, and ideally, we'd have some image level flag that informs the loader whether all code sections in the image were emitted with guard instructions for BTI/IBT. We need more code to detect *all* runtime images. The logic could be: 1) PE/COFF loader (X86, ARM) detects IBT capability for all runtime images,= and report it. 2) DXE Core collects such info. 2.1) If ALL runtime image supports IBT, then gMemoryAttributesTableForwardC= fi =3D TRUE 2.2) Else, gMemoryAttributesTableForwardCfi =3D FALSE. The benefit is that it is more reliable to support binary PE image use case= . Hard to use build time flag for external binary PE ... Agreed. This sounds like a good way to implement it. -----Original Message----- From: Michael Kubacki > Sent: Friday, February 3, 2023 8:24 AM To: devel@edk2.groups.io; ardb@kernel.org; Kinney, Michael D > Cc: Gao, Liming >= ; Yao, Jiewen >; Kubacki, Michael >; Sean Brogan >; = Rebecca Cran >; Leif Lindholm >; Sami Mujawar >; Taylor Beebe <= t@taylorbeebe.com> Subject: Re: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in mem attributes table Ard, I am still actively tracking this for the PE/COFF spec. Unfortunately, I don't have more firm info right now but I suggest holding off on alternatives for the time being and I will reply back as soon as the next steps are known. Thanks, Michael On 2/2/2023 2:00 PM, Ard Biesheuvel wrote: On Thu, 2 Feb 2023 at 19:49, Kinney, Michael D > wrote: Hi Ard, Since the PE/COFF image does not contain this information, is there an option to add the information to an FFS file. Either as a new bit in a standard he= ader or as a GUIDed section defined by EDK II? Since an FV may contain content build from source and additional content Integrated as binaries from other vendors, having a PCD that scopes this attribute to all FVs many only work for FW builds that are 100% from source= s. I didn't quite consider that scenario tbh. It would be nice if we could avoid another PI spec dance for this, and get some kind of IBT/BTI annotation added to the PE/COFF spec. That way, we cover this issue, as well as clear the way for the use if IBT/BTI when running under the firmware. Are TE images a concern here as well? I.e., are those likely to be used for runtime DXE images in firmware volumes? -----Original Message----- From: Ard Biesheuvel > Sent: Thursday, February 2, 2023 10:04 AM To: devel@edk2.groups.io Cc: Ard Biesheuvel >; Kinney, Micha= el D >; Gao, Limin= g >; Yao, Jiewen >; Kubacki, Michae= l >; Sean= Brogan >; Rebecca Cran >; Leif Lindholm >; Sami Mujawar= >; Taylor Beebe > Subject: [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in mem attributes table The memory attributes table has been extended with a flag that indicates whether or not the OS is permitted to map the EFI runtime code regions with strict enforcement for IBT/BTI landing pad instructions. This is generally a property of the firmware build, and so we can permit a platform to set this property using a PCD, and put the burden on the platform definition to set the toolchain options accordingly. There is one snag, however: PE/COFF does not expose whether or not the code was generated with landing pads, so if any runtime DXE drivers were loaded from storage other than the firmware volumes, we must assume that setting the CFI flag in the memory attributes table is unsafe. Signed-off-by: Ard Biesheuvel > --- MdeModulePkg/Core/Dxe/DxeMain.h | 2 ++ MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + MdeModulePkg/Core/Dxe/Image/Image.c | 11 +++++++++++ MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 7 ++++++- MdeModulePkg/MdeModulePkg.dec | 8 ++++++++ 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h index 815a6b4bd844..427a5fc78f72 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.h +++ b/MdeModulePkg/Core/Dxe/DxeMain.h @@ -280,6 +280,8 @@ extern EFI_MEMORY_TYPE_INFORMATION gMemoryTypeInformation[EfiMaxMemoryType + 1] extern BOOLEAN gDispatcherRunning; extern EFI_RUNTIME_ARCH_PROTOCOL gRuntimeTemplate; +extern BOOLEAN gMemoryAttributesTableForwardCfi; + extern EFI_LOAD_FIXED_ADDRESS_CONFIGURATION_TABLE gLoadModuleAtFixAddressConfigurationTable; extern BOOLEAN gLoadFixedAddressCodeMemoryReady; // diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index 35d5bf0dee6f..e6ff67773a69 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -187,6 +187,7 @@ [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryAttributesTableForwardCfi ## CONSUMES # [Hob] # RESOURCE_DESCRIPTOR ## CONSUMES diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c index 06cc6744b8c6..181fefdb6657 100644 --- a/MdeModulePkg/Core/Dxe/Image/Image.c +++ b/MdeModulePkg/Core/Dxe/Image/Image.c @@ -1398,6 +1398,17 @@ CoreLoadImageCommon ( CoreNewDebugImageInfoEntry (EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL, &Image->Info, Image->Handle); } + // + // If we loaded a runtime DXE driver from something other than a FV, it + // was not built as part of the firmware image, and so we cannot assume + // that it was built with IBT/BTI landing pads for forward edge control + // flow integrity. + // + if (!ImageIsFromFv && + (Image->ImageContext.ImageCodeMemoryType =3D=3D EfiRuntimeServicesCode)) { + gMemoryAttributesTableForwardCfi =3D FALSE; + } + // // Reinstall loaded image protocol to fire any notifications // diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c index e07921371187..cdd35ade0a8a 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c @@ -89,6 +89,7 @@ BOOLEAN mMemoryAttributesTableEnable =3D TRUE; BOOLEAN mMemoryAttributesTableEndOfDxe =3D FALSE; EFI_MEMORY_ATTRIBUTES_TABLE *mMemoryAttributesTable =3D NULL; BOOLEAN mMemoryAttributesTableReadyToBoot =3D FALSE; +BOOLEAN gMemoryAttributesTableForwardCfi =3D FixedPcdGetBool (PcdMemoryAttributesTableForwardCfi); /** Install MemoryAttributesTable. @@ -182,7 +183,11 @@ InstallMemoryAttributesTable ( MemoryAttributesTable->Version =3D EFI_MEMORY_ATTRIBUTES_TABLE_VERSION; MemoryAttributesTable->NumberOfEntries =3D RuntimeEntryCount; MemoryAttributesTable->DescriptorSize =3D (UINT32)DescriptorSize; - MemoryAttributesTable->Reserved =3D 0; + if (gMemoryAttributesTableForwardCfi) { + MemoryAttributesTable->Flags =3D EFI_MEMORY_ATTRIBUTES_FLAGS_RT_FORWARD_CONTROL_FLOW_GUARD; + } else { + MemoryAttributesTable->Flags =3D 0; + } DEBUG ((DEBUG_VERBOSE, "MemoryAttributesTable:\n")); DEBUG ((DEBUG_VERBOSE, " Version - 0x%08x\n", MemoryAttributesTable->Version)); DEBUG ((DEBUG_VERBOSE, " NumberOfEntries - 0x%08x\n", MemoryAttributesTable->NumberOfEntries)); diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 9605c617b7a8..d336a38655a3 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1093,6 +1093,14 @@ [PcdsFixedAtBuild] # @Prompt Enable UEFI Stack Guard. gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30 001055 + ## Indicates whether the EFI memory attributes table will inform the OS that + # forward edge control flow guards have been inserted into the runtime services + # code regions. + # TRUE - Runtime code has forward control flow guards.
+ # FALSE - Runtime code does not have forward control flow guards.
+ # @Prompt Enable forward control flow guards in EFI memory attributes table + gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryAttributesTableForwardCfi|FAL SE|BOOLEAN|0x30001056 + [PcdsFixedAtBuild, PcdsPatchableInModule] ## Dynamic type PCD can be registered callback function for Pcd setting action. # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function -- 2.39.1 --_000_MW4PR11MB58726E1FBE8D9206D8B9137C8CD79MW4PR11MB5872namp_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

Adding bit in Image header is best way. I totally ag= ree.

The only disadvantage is that it may take time to up= date PE/COFF specification and take time to update the compiler to generate= such bit.

If people want to wait for those spec update, I don&= #8217;t have any concern.

 

 

Personally, I don’t think adding bit in FFS is= a good idea. An image may come from anywhere – Flash, PCI card, Disk= , Network. Hard to scale.

 

 

Thank you

Yao, Jiewen

 

From: devel@edk2.groups.io <devel@edk2.gro= ups.io> On Behalf Of Marvin H=E4user
Sent: Friday, February 3, 2023 5:52 PM
To: Ard Biesheuvel <ardb@kernel.org>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forwa= rd edge CFI in mem attributes table

 

Hi Ard and Jiewen,

(I‘m replying from groups.io and cannot figure out how to CC Jiewen. = Ugh.)

Personally, I‘d rather have UEFI itself rely solely on the flag in th= e image file. If there is a way needed to handle images without the tag, in= my opinion use some userland preprocessing tool to check and annotate it w= hen generating the firmware image. Even if it‘s trivial for Intel, I hope we won‘t end up with a micro= -disassembler in firmware just to tell whether a feature is enabled.

Best regards,
Marvin


On Fri, Feb 3, 2023 at 12:26 AM, Ard Biesheuvel wrote:

(cc Samer, Jose)

On Fri, 3 Feb 2023 at 02:16, Yao, Jiewen <jiewen.yao@intel.com> wrote:


Hello
Can we assume that the entrypoint of PE/COFF image is always ENDBR64, if th= e PE/COFF image is enlightened to support IBT?

I believe the compiler should do that, because the loader need use indirect= call to the PE/COFF entrypoint.

That is an interestin= g idea. Even if we have some PE level annotation
for BTI/IBT at some point, it would be a good sanity check on the
image, because if the entry point does not have the guard instruction,
there is obviously something wrong.

However, on arm64, it is a bit more ambiguous, because there are
instructions for pointer authentication (PACIASP) that may be
interpreted as implicit guard instructions too, so the presence of
such an instruction at the entry point does not imply that BTI is
enabled in the entire image.

I'll code up a PoC with this, but us ARM folks should have a think
about how to spec this out and deploy this. I wonder

In the mean time, Michael is trying to round up the MS folks that are
involved in maintaining the PE/COFF spec, and ideally, we'd have some
image level flag that informs the loader whether all code sections in
the image were emitted with guard instructions for BTI/IBT.


We need more code to detect *all* runtime images. Th= e logic could be:
1) PE/COFF loader (X86, ARM) detects IBT capability for all runtime images,= and report it.
2) DXE Core collects such info.
2.1) If ALL runtime image supports IBT, then gMemoryAttributesTableForwardC= fi =3D TRUE
2.2) Else, gMemoryAttributesTableForwardCfi =3D FALSE.

The benefit is that it is more reliable to support binary PE image use case= . Hard to use build time flag for external binary PE ...

Agreed. This sounds l= ike a good way to implement it.

-----Original Message-----
From: Michael Kubacki <m= ikuback@linux.microsoft.com>
Sent: Friday, February 3, 2023 8:24 AM
To: devel@edk2.groups.io; ardb@kernel.org; Kinney, Michael D
<michael.d.kinney@intel.co= m>
Cc: Gao, Liming <gaoliming@b= yosoft.com.cn>; Yao, Jiewen
<jiewen.yao@intel.com>; K= ubacki, Michael <michae= l.kubacki@microsoft.com>;
Sean Brogan <sean.brogan@mi= crosoft.com>; Rebecca Cran
<quic_rcran@quicinc.com>= ;; Leif Lindholm <quic_llin= dhol@quicinc.com>; Sami
Mujawar <sami.mujawar@arm.com>; Taylor Beebe <t@taylorbeebe.= com>
Subject: Re: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forward edge=
CFI in mem attributes table

Ard, I am still actively tracking this for the PE/COFF spec.

Unfortunately, I don't have more firm info right now but I suggest
holding off on alternatives for the time being and I will reply back as
soon as the next steps are known.

Thanks,
Michael

On 2/2/2023 2:00 PM, Ard Biesheuvel wrote:

On Thu, 2 Feb 2023 at 19:49, Kinney, Michael D
<michael.d.kinney@intel.co= m> wrote:


Hi Ard,

Since the PE/COFF image does not contain this information, is there an=

option

to add the information to an FFS file. Either as a n= ew bit in a standard header
or as a GUIDed section defined by EDK II?

Since an FV may contain content build from source and additional content Integrated as binaries from other vendors, having a PCD that scopes this attribute to all FVs many only work for FW builds that are 100% from source= s.

I didn't quite consid= er that scenario tbh.

It would be nice if we could avoid another PI spec dance for this, and
get some kind of IBT/BTI annotation added to the PE/COFF spec. That
way, we cover this issue, as well as clear the way for the use if
IBT/BTI when running under the firmware.

Are TE images a concern here as well? I.e., are those likely to be
used for runtime DXE images in firmware volumes?

 

-----Original Message-----
From: Ard Biesheuvel <ardb@kernel.org= >
Sent: Thursday, February 2, 2023 10:04 AM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb@kernel.org>; Kinney, Michael D

<mi= chael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Yao,

Jiewen <j= iewen.yao@intel.com>; Kubacki, Michael

<michael.kubacki@microsoft.com>; Sean Brogan
<sean.brogan@microsoft.com<= /a>>; Rebecca

Cran <q= uic_rcran@quicinc.com>; Leif Lindholm

<qui= c_llindhol@quicinc.com>; Sami Mujawar <sami.mujawar@arm.com>;
Taylor Beebe

<t@taylorbee= be.com>
Subject: [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in mem<= /o:p>

attributes table


The memory attributes table has been extended with a flag that indicates whether or not the OS is permitted to map the EFI runtime code regions
with strict enforcement for IBT/BTI landing pad instructions.

This is generally a property of the firmware build, and so we can permit a platform to set this property using a PCD, and put the burden on the
platform definition to set the toolchain options accordingly.

There is one snag, however: PE/COFF does not expose whether or not the
code was generated with landing pads, so if any runtime DXE drivers were loaded from storage other than the firmware volumes, we must assume

that

setting the CFI flag in the memory attributes table = is unsafe.

Signed-off-by: Ard Biesheuvel <ardb@k= ernel.org>
---
MdeModulePkg/Core/Dxe/DxeMain.h | 2 ++
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/Image/Image.c | 11 +++++++++++
MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 7 ++++++-
MdeModulePkg/MdeModulePkg.dec | 8 ++++++++
5 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h

b/MdeModulePkg/Core/Dxe/DxeMain.h

index 815a6b4bd844..427a5fc78f72 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -280,6 +280,8 @@ extern EFI_MEMORY_TYPE_INFORMATION

gMemoryTypeInformation[EfiMaxMemoryType + 1]

extern BOOLEAN gDispatcherRunning;

extern EFI_RUNTIME_ARCH_PROTOCOL gRuntimeTemplate;



+extern BOOLEAN gMemoryAttributesTableForwardCfi;

+

extern EFI_LOAD_FIXED_ADDRESS_CONFIGURATION_TABLE

gLoadModuleAtFixAddressConfigurationTable;


extern BOOLEAN

gLoadFixedAddressCodeMemoryReady;


//

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf

b/MdeModulePkg/Core/Dxe/DxeMain.inf

index 35d5bf0dee6f..e6ff67773a69 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -187,6 +187,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask

## CONSUMES


gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ##

CONSUMES

gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsu= lationDepth
## CONSUMES


+

gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryAttributesTa= bleForwardCfi
## CONSUMES




# [Hob]

# RESOURCE_DESCRIPTOR ## CONSUMES

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c

b/MdeModulePkg/Core/Dxe/Image/Image.c

index 06cc6744b8c6..181fefdb6657 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -1398,6 +1398,17 @@ CoreLoadImageCommon (
CoreNewDebugImageInfoEntry

(EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL, &Image->In= fo, Image->Handle);


}



+ //

+ // If we loaded a runtime DXE driver from something other than a FV, it
+ // was not built as part of the firmware image, and so we cannot assume
+ // that it was built with IBT/BTI landing pads for forward edge control
+ // flow integrity.

+ //

+ if (!ImageIsFromFv &&

+ (Image->ImageContext.ImageCodeMemoryType =3D=3D

EfiRuntimeServicesCode)) {


+ gMemoryAttributesTableForwardCfi =3D FALSE;

+ }

+

//

// Reinstall loaded image protocol to fire any notifications

//

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c<= /p>

b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c=

index e07921371187..cdd35ade0a8a 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -89,6 +89,7 @@ BOOLEAN mMemoryAttributesTableEnable

=3D TRUE;

BOOLEAN mMemoryAttributesTableEndOfDxe =3D FALSE;
EFI_MEMORY_ATTRIBUTES_TABLE *mMemoryAttributesTable =3D

NULL;


BOOLEAN mMemoryAttributesTableReadyToBoot =3D FALSE;

+BOOLEAN gMemoryAttributesTableForwardCfi =3D

FixedPcdGetBool (PcdMemoryAttributesTableForwardCfi)= ;




/**

Install MemoryAttributesTable.

@@ -182,7 +183,11 @@ InstallMemoryAttributesTable (
MemoryAttributesTable->Version =3D

EFI_MEMORY_ATTRIBUTES_TABLE_VERSION;


MemoryAttributesTable->NumberOfEntries =3D RuntimeEntryCount;

MemoryAttributesTable->DescriptorSize =3D (UINT32)DescriptorSize;

- MemoryAttributesTable->Reserved =3D 0;

+ if (gMemoryAttributesTableForwardCfi) {

+ MemoryAttributesTable->Flags =3D

EFI_MEMORY_ATTRIBUTES_FLAGS_RT_FORWARD_CONTROL_FLOW_= GUARD;


+ } else {

+ MemoryAttributesTable->Flags =3D 0;

+ }

DEBUG ((DEBUG_VERBOSE, "MemoryAttributesTable:\n"));

DEBUG ((DEBUG_VERBOSE, " Version - 0x%08x\n",

MemoryAttributesTable->Version));


DEBUG ((DEBUG_VERBOSE, " NumberOfEntries - 0x%08x\n",<= /p>

MemoryAttributesTable->NumberOfEntries));


diff --git a/MdeModulePkg/MdeModulePkg.dec

b/MdeModulePkg/MdeModulePkg.dec

index 9605c617b7a8..d= 336a38655a3 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1093,6 +1093,14 @@ [PcdsFixedAtBuild]
# @Prompt Enable UEFI Stack Guard.

gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALS= E|BOOLEAN|0x30
001055




+ ## Indicates whether the EFI memory attributes table will inform the OS

that


+ # forward edge control flow guards have been inserted into the runtime

services


+ # code regions.

+ # TRUE - Runtime code has forward control flow guards.<BR>

+ # FALSE - Runtime code does not have forward control flow guards.<BR&g= t;

+ # @Prompt Enable forward control flow guards in EFI memory attributes

table


+

gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryAttributesTa= bleForwardCfi|FAL
SE|BOOLEAN|0x30001056


+

[PcdsFixedAtBuild, PcdsPatchableInModule]

## Dynamic type PCD can be registered callback function for Pcd setting

action.


# PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum

number of callback function


--
2.39.1

 

--_000_MW4PR11MB58726E1FBE8D9206D8B9137C8CD79MW4PR11MB5872namp_--