Updated version #4. > -----Original Message----- > From: Sandra Loosemore [mailto:san...@codesourcery.com] > Sent: Wednesday, September 27, 2017 5:11 AM > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; 'gcc- > patc...@gcc.gnu.org' <gcc-patches@gcc.gnu.org> > Cc: Jeff Law <l...@redhat.com> > Subject: Re: 0002-Part-2.-Document-finstrument-control-flow-and-notrack > attribute > > On 09/26/2017 07:45 AM, Tsimbalist, Igor V wrote: > > Here is the updated version (version#3). All comments below are fixed. > > This still needs more work. Specific comments below: > > > +The @code{nocf_check} attribute is applied to an object's type. > > +In case of assignment of a function address or a function pointer to > > +another pointer, the attribute is not carried over from the > > +right-hand object's type, the type of left-hand object stays > > +unchanged. The > > s/object's type,/object's type;/
Fixed.
> > @@ -11348,6 +11349,35 @@ is used to link a program, the GCC driver
> > automatically links against @file{libmpxwrappers}. See also @option{-
> static-libmpxwrappers}.
> > Enabled by default.
> >
> > +@item -fcf-
> protection==@r{[}full@r{|}branch@r{|}return@r{|}none@r{]}
> > +@opindex fcf-protection
> > +Enable code instrumentation of control-flow transfers to increase
> > +program security by checking that target addresses of control-flow
> > +transfer instructions (such as indirect function call, function
> > +return, indirect jump) are valid. This prevents diverting the
> > +control flow instructions from its original target address to a new
> > +undesigned
>
> s/control flow instructions/control-flow instructions/
>
> I'd rewrite the next sentence as
>
> This prevents diverting the flow of control to an unexpected target.
I used your suggestion.
> > +target. This is intended to protect against such threats as
> > +Return-oriented Programming (ROP), and similarly call/jmp-oriented
> > +programming (COP/JOP).
> > +
> > +Each compiler target, which is going to support the control-flow
> > +instrumentation, is supposed to have its own target specific
> > +implementation. For all targets where an implementation is absent the
> > +usage of @option{-fcf-protection} option causes an error message.
>
> I would really prefer that you list the targets this works on here instead.
Another patch you are reviewing now (its name starts with 0005-Part-5)
has the statement you would like to put here. The important point here is
an error issuing. When I commit the first patch none of target platforms
supports
the option and an error is printed when the option is specified. I removed the
first sentence but keep the second one:
For all targets, which do not support the @option{-fcf-protection}
option, the option usage results in an error message.
> > +The value @code{branch} tells the compiler to implement checking of
> > +validity of control-flow transfer at the point of indirect branch
> > +instructions, i.e. call/jmp instructions. The value @code{return}
> > +implements checking of validity at the point of returning from a
> > +function. The value @code{full} is an alias for specifying both
> > +@code{branch} and @code{return}. The value @code{none} turns off
> > +instrumentation. This value may be used for future architectures
> > +where @option{-fcf-protection} option is switched on by default.
>
> I don't think we need to document GCC's future behavior for future
> architectures (I'm always going around removing useless discussion from
> 20 years ago of possible extensions that never got implemented). I assume
> that this is just provided for completeness and to override a previous -fcf-
> protection option on the command line.
Ok, removed the last sentence.
> > +You can also use the @code{nocf_check} attribute to identify which
> > +functions and calls should be skipped from instrumentation
> > +(@pxref{Function Attributes}).
> > +
> > @item -fstack-protector
> > @opindex fstack-protector
> > Emit extra code to check for buffer overflows, such as stack smashing
> > diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi index
> > 12355c2..b4fc5f3 100644
> > --- a/gcc/doc/rtl.texi
> > +++ b/gcc/doc/rtl.texi
> > @@ -4040,6 +4040,22 @@ is used in place of the actual insn pattern.
> > This is done in cases where the pattern is either complex or misleading.
> > @end table
> >
> > +The note @code{REG_CALL_NOCF_CHECK} is used in conjunction with
> the
> > +@option{-fcf-protection=branch} option. The note is set if a
> > +@code{nocf_check} attribute is specified for a function type or a
> > +pointer to function type. The note is stored in the @code{REG_NOTES}
> > +field of an insn.
> > +
> > +@table @code
> > +@findex REG_CALL_NOCF_CHECK
> > +@item REG_CALL_NOCF_CHECK
> > +A user has a control through the @code{nocf_check} attribute to
> > +identify
>
> S/A user has a control/Users have control/
Fixed.
> > +which call to a function should be skipped from control-flow
> > +instrumentation
>
> s/call/calls/
Fixed.
> > +when the option @option{-fcf-protection=branch} is specified. The
> > +compiler puts a @code{REG_CALL_NOCF_CHECK} note on
> @code{CALL_INSN}
> > +instruction, which has a function type marked with a @code{nocf_check}
> attribute.
>
> s/@code{CALL_INSN} instruction, which/each @code{CALL_INSN}
> instruction that/
Fixed.
Igor
> -Sandra
0002-Add-documentation-for-fcf-protection-option-and-nocf.patch
Description: 0002-Add-documentation-for-fcf-protection-option-and-nocf.patch
