0001-Part-1.-Add-generic-part-for-Intel-CET-enabling

[Tsimbalist, Igor V]

> -----Original Message-----
> From: Richard Biener [mailto:richard.guenther@gmail.com]
> Sent: Friday, August 18, 2017 3:53 PM
> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> 
> On Fri, Aug 18, 2017 at 3:11 PM, Tsimbalist, Igor V
> <igor.v.tsimbalist@intel.com> wrote:
> >> -----Original Message-----
> >> From: Richard Biener [mailto:richard.guenther@gmail.com]
> >> Sent: Tuesday, August 15, 2017 3:43 PM
> >> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
> >>
> >> On Tue, Aug 1, 2017 at 10:56 AM, Tsimbalist, Igor V
> >> <igor.v.tsimbalist@intel.com> wrote:
> >> > Part#1. Add generic part for Intel CET enabling.
> >> >
> >> > The spec is available at
> >> >
> >> > https://software.intel.com/sites/default/files/managed/4d/2a/contro
> >> > l-f low-enforcement-technology-preview.pdf
> >> >
> >> > High-level design.
> >> > ------------------
> >> >
> >> > A proposal is to introduce a target independent flag
> >> > -finstrument-control-flow with a semantic to instrument a code to
> >> > control validness or integrity of control-flow transfers using jump
> >> > and call instructions. The main goal is to detect and block a
> >> > possible malware execution through transfer the execution to
> >> > unknown target address. Implementation could be either software or
> >> > target based. Any target platforms can provide their implementation
> >> > for instrumentation under this option.
> >> >
> >> > When the -finstrument-control-flow flag is set each implementation
> >> > has to check if a support exists for a target platform and report
> >> > an error if no support is found.
> >> >
> >> > The compiler should instrument any control-flow transfer points in
> >> > a program (ex. call/jmp/ret) as well as any landing pads, which are
> >> > targets of for control-flow transfers.
> >> >
> >> > A new 'notrack' attribute is introduced to provide hand tuning support.
> >> > The attribute directs the compiler to skip a call to a function and
> >> > a function's landing pad from instrumentation (tracking). The
> >> > attribute can be used for function and pointer to function types,
> >> > otherwise it will be ignored. The attribute is saved in a type and
> >> > propagated to a GIMPLE call statement and later to a call instruction.
> >> >
> >> > Currently all platforms except i386 will report the error and do no
> >> > instrumentation. i386 will provide the implementation based on a
> >> > specification published by Intel for a new technology called
> >> > Control-flow Enforcement Technology (CET).
> >>
> >> diff --git a/gcc/gimple.c b/gcc/gimple.c index 479f90c..2e4ab2d
> >> 100644
> >> --- a/gcc/gimple.c
> >> +++ b/gcc/gimple.c
> >> @@ -378,6 +378,23 @@ gimple_build_call_from_tree (tree t)
> >>    gimple_set_no_warning (call, TREE_NO_WARNING (t));
> >>    gimple_call_set_with_bounds (call, CALL_WITH_BOUNDS_P (t));
> >>
> >> +  if (fndecl == NULL_TREE)
> >> +    {
> >> +      /* Find the type of an indirect call.  */
> >> +      tree addr = CALL_EXPR_FN (t);
> >> +      if (TREE_CODE (addr) != FUNCTION_DECL)
> >> +       {
> >> +         tree fntype = TREE_TYPE (addr);
> >> +         gcc_assert (POINTER_TYPE_P (fntype));
> >> +         fntype = TREE_TYPE (fntype);
> >> +
> >> +         /* Check if its type has the no-track attribute and propagate
> >> +            it to the CALL insn.  */
> >> +         if (lookup_attribute ("notrack", TYPE_ATTRIBUTES (fntype)))
> >> +           gimple_call_set_with_notrack (call, TRUE);
> >> +       }
> >> +    }
> >>
> >> this means notrack is not recognized if fndecl is not NULL.  Note
> >> that only the two callers know the real function type in effect (they
> >> call gimple_call_set_fntype with it).  I suggest to pass down that
> >> type to gimple_build_call_from_tree and move the
> >> gimple_call_set_fntype call there as well.  And simply use the type for the
> above.
> >
> > The best way to say is notrack is not propagated if fndecl is not NULL.
> Fndecl, if not NULL, is a direct call and notrack is not applicable for such calls. I
> will add a comment before the if.
> 
> Hmm.  But what does this mean?  I guess direct calls are always 'notrack' then
> and thus we're fine to ignore it.

Yes, that's correct - direct calls are always 'notrack' and we are ignoring it in calls.

> > I would like to propose modifying the existing code without changing
> interfaces. The idea is that at the time the notrack is propagated (the code
> snippet above) the gimple call was created and the correct type was assigned
> to the 'call' exactly by gimple_call_set_fntype. My proposal is to get the type
> out of the gimple 'call' (like gimple_call_fntype) instead of the tree 't'. Is it
> right?
> 
> Yes, but note that the type on the gimple 'call' isn't yet correctly set (only the
> caller does that).  The gimple_build_call_from_tree is really an ugly thing and
> it should be privatized inside the gimplifier...

I'll look into this more carefully. It looks like it has to be rewritten as you suggested.

> >> +static inline bool
> >> +gimple_call_with_notrack_p (const gimple *gs) {
> >> +  const gcall *gc = GIMPLE_CHECK2<const gcall *> (gs);
> >> +  return gimple_call_with_notrack_p (gc); }
> >>
> >> please do not add gimple * overloads for new APIs, instead make sure
> >> to pass down gcalls at callers.
> >
> > Ok, I will remove.
> >
> >> Please change the names to omit 'with_', thus just notrack and
> >> GF_CALL_NOTRACK.
> >
> > Ok, I will rename.
> >
> >> I think 'notrack' is somewhat unspecific of a name, what prevented
> >> you to use 'nocet'?
> >
> > Actually it's specific. The HW will have a prefix with exactly this name and
> the same meaning. And I think, what is more important, 'track/notrack' gives
> better semantic for a user. CET is a name bound with Intel specific
> technology.
> 
> But 'tracking' something is quite unspecific.  Tracking for what?
> 'no_verify_cf' (aka do not verify control flow) maybe?

The name just  has to suggest the right semantic. 'no_verify_cf' is good, let's use it
unless different name appears.

> >> Any idea how to implement a software-based solution efficiently?
> >> Creating a table of valid destination addresses in a special section
> >> should be possible without too much work, am I right in that only
> >> indirect control transfer is checked?  Thus CET assumes the code
> >> itself cannot be changed (and thus the stack isn't executable)?
> >
> > Yes, your idea looks right. Microsoft has published the option /guard:cf and
> according to the description it uses similar idea of gathering information
> about control-transfer targets. This info is kept in the binary. And 'yes' for last
> two your questions.
> 
> Ok, so if we generate a special section with say 'cettable' as symbol the
> verification runtime would be invoked at each indirect call site with the
> destination address and it would search the table and abort if the destination
> is not recorded.  We probably have to support a set of CET tables and register
> them to support shared libraries (and cross shared object indirect calls).
> 
> Do you plan to contribute sth like this?  Maybe within the sanitizer
> framework?

I do not have plans for any sw based implementation. My plan is to introduce the
concept in gcc and make its enabling for Intel CET. Even with my current plans more work
needs to be done to completely enable CET.

Thanks,
Igor

> Thanks,
> Richard.
> 
> > Thanks,
> > Igor
> >
> >> Thanks,
> >> Richard.