Re: [PATCH v2 00/70] x86: Support for CET Indirect Branch Tracking

From: Andrew Cooper <Andrew.Cooper3@citrix.com>
To: Jan Beulich <jbeulich@suse.com>,
	Roger Pau Monne <roger.pau@citrix.com>, Wei Liu <wl@xen.org>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Juergen Gross" <jgross@suse.com>,
	"Daniel Smith" <dpsmith@apertussolutions.com>,
	"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v2 00/70] x86: Support for CET Indirect Branch Tracking
Date: Wed, 16 Feb 2022 21:59:47 +0000	[thread overview]
Message-ID: <9bcbd2f9-f713-cb56-abdd-baef5180a964@citrix.com> (raw)
In-Reply-To: <83e15c5d-f48b-a6f2-ebca-e416bdef1093@suse.com>

On 14/02/2022 14:38, Jan Beulich wrote:
> On 14.02.2022 15:15, Andrew Cooper wrote:
>> On 14/02/2022 13:43, Jan Beulich wrote:
>>> On 14.02.2022 14:10, Andrew Cooper wrote:
>>>> On 14/02/2022 12:50, Andrew Cooper wrote:
>>>>> CET Indirect Branch Tracking is a hardware feature designed to protect against
>>>>> forward-edge control flow hijacking (Call/Jump oriented programming), and is a
>>>>> companion feature to CET Shadow Stacks added in Xen 4.14.
>>>>>
>>>>> Patches 1 thru 5 are prerequisites.  Patches 6 thru 60 are fairly mechanical
>>>>> annotations of function pointer targets.  Patches 61 thru 70 are the final
>>>>> enablement of CET-IBT.
>>>>>
>>>>> This series functions correctly with GCC 9 and later, although an experimental
>>>>> GCC patch is required to get more helpful typechecking at build time.
>>>>>
>>>>> Tested on a TigerLake NUC.
>>>>>
>>>>> CI pipelines:
>>>>>   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/470453652
>>>>>   https://cirrus-ci.com/build/4962308362338304
>>>>>
>>>>> Major changes from v1:
>>>>>  * Boilerplate for mechanical commits
>>>>>  * UEFI runtime services unconditionally disable IBT
>>>>>  * Comprehensive build time check for embedded endbr's
>>>> There's one thing I considered, and wanted to discuss.
>>>>
>>>> I'm tempted to rename cf_check to cfi for the function annotation, as
>>>> it's shorter without reducing clarity.
>>> What would the 'i' stand for in this acronym?
>> The class of techniques is called Control Flow Integrity.
>>
>>>  Irrespective of the answer
>>> I'd like to point out the name collision with the CFI directives at
>>> assembler level. This isn't necessarily an objection (I'm certainly for
>>> shortening), but we want to avoid introducing confusion.
>> I doubt there is confusion to be had here.  One is entirely a compiler
>> construct which turns into ENDBR64 instructions in the assembler, and
>> one is a general toolchain construct we explicitly disable.
> Hmm. I'm still at best half convinced. Plus we generally have been
> naming our shorthands after the actual attribute names. By using
> "cfi" such a connection would also be largely lost. Roger, Wei,
> others - do you opinions either way?

My point is this.  Doing nothing is my easiest option.

But if anyone has length/alternative suggestions, dealing with them now
is going to be infinitely easier than once this series is committed.

~Andrew