Message ID | 20220214125127.17985-1-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
Headers | show |
Series | x86: Support for CET Indirect Branch Tracking | expand |
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. Changing now (i.e. before I commit) is easy. Once committed, changing is far harder. ~Andrew
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? 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. Jan
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. ~Andrew
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? Jan
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
On 16.02.2022 22:59, Andrew Cooper wrote: > 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. Understood. My personal preference then is to stick with cf_check. Jan