[PATCH] x86: Allow -fcf-protection with multi-byte NOPs

Tsimbalist, Igor V igor.v.tsimbalist@intel.com
Thu Apr 19 14:08:00 GMT 2018


> -----Original Message-----
> From: Uros Bizjak [mailto:ubizjak@gmail.com]
> Sent: Thursday, April 19, 2018 3:36 PM
> To: H.J. Lu <hjl.tools@gmail.com>
> Cc: Richard Biener <richard.guenther@gmail.com>; gcc-
> patches@gcc.gnu.org; Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Subject: Re: [PATCH] x86: Allow -fcf-protection with multi-byte NOPs
> 
> On Thu, Apr 19, 2018 at 3:30 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Wed, Apr 18, 2018 at 01:35:33PM +0200, Richard Biener wrote:
> >> On Wed, Apr 18, 2018 at 1:24 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> >> > On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com>
> wrote:
> >> >> On Tue, Apr 17, 2018 at 12:25 PM, H.J. Lu <hjl.tools@gmail.com>
> wrote:
> >> >>> On Tue, Apr 17, 2018 at 12:03 PM, H.J. Lu <hjl.tools@gmail.com>
> wrote:
> >> >>>> On Tue, Apr 17, 2018 at 11:55 AM, Uros Bizjak
> <ubizjak@gmail.com> wrote:
> >> >>>>> On Tue, Apr 17, 2018 at 8:42 PM, H.J. Lu
> <hongjiu.lu@intel.com> wrote:
> >> >>>>>> -fcf-protection -mcet can't be used with IFUNC features, like
> symbol
> >> >>>>>> multiversioning or target clone, since IBT/SHSTK are applied to
> the whole
> >> >>>>>> program and they may be disabled in some functions.  But -
> fcf-protection
> >> >>>>>> is implemented with multi-byte NOPs on all 64-bit processors
> as well as
> >> >>>>>> 32-bit processors starting with Pentium Pro.  If -fcf-protection
> requires
> >> >>>>>> -mcet, IFUNC features can't be used on Linux when -fcf-
> protection is
> >> >>>>>> enabled by default.
> >> >>>>>>
> >> >>>>>> This patch changes -fcf-protection to to enable the NOP
> portion of CET
> >> >>>>>> ISAs unless IBT and/or SHSTK are disabled explicitly.  The rest of
> CET
> >> >>>>>> ISAs, including intrinsics, still requires -mcet, -mibt or -mshstk.
> >> >>>>>>
> >> >>>>>> OK for trunk?
> >> >>>>>
> >> >>>>> As said in the PR, NOP sequences have non-zero cost in the
> executable
> >> >>>>> (they enlarge the executable), so I don't think this feature should
> be
> >> >>>>> enabled by default.
> >> >>>>>
> >> >>>>> There is always a configure option if someone wants their
> compiler to
> >> >>>>> always emit relevant multi-byte nops.
> >> >>>>
> >> >>>> What we need is an option to enable -fcf-function with multi-byte
> NOPs
> >> >>>> without -mcet which enables the full CET ISAs.  A configure option
> >> >>>> without the corresponding the command-line option makes test
> and
> >> >>>> debug difficult.   I can add
> >> >>>>
> >> >>>> --enable-cf-function-nop or --with-cf-function-nop
> >> >>>>
> >> >>>> with
> >> >>>>
> >> >>>> -fct-function-nop
> >> >>>>
> >> >>>
> >> >>> How about adding -mno-cet, which enables the NOP portion of
> CET
> >> >>
> >> >> I meant -mnop-cet, not -mno-cet.
> >> >>
> >> >
> >> > Here is a patch to add -mnop and use it with -fcf-protection.
> >>
> >> +mnop
> >> +Target Report Var(flag_nop) Init(0)
> >> +Support multi-byte NOP code generation.
> >>
> >> the option name is incredibly bad and the documentation doesn't make it
> >> better either.  The invoke.texi docs refer to duplicate {-mcet}.
> >>
> >> Isn't there a -fcf-protection sub-set that can be used to automatically
> >> enable this?  Or simply do this mode by default when
> >> -fcf-protection is used but neither -mcet nor -mibt is enabled?
> >>
> >
> > Since multi-byte NOPs are used to implement -fcf-protection on x86, we
> > propose a new design for -fcf-protection:
> >
> > 1. -fcf-protection option will report the unsupported error on non-x86
> > platform. On x86 platform it's supported and inserts endbr-nop
> > instructions and properties, depending on its value (full/branch/return)
> > 2. -mcet/-mibt/-mshstk options control intrinsics only.
> > 3. These options are independent and do not influence each other so no
> > need for cross checking between them.
> >
> > OK for trunk?
> 
> This patch touches only CET related code, so Igor's OK should be enough.

I have reviewed the patch and I'm ok with it.

Igor

> Uros.


More information about the Gcc-patches mailing list