Re: [PATCH] Call REAL(swapcontext) with indirect_return attribute on x86

["Kostya Serebryany via gcc-patches" <gcc-patches at gcc dot gnu dot org>]

On Wed, Jul 18, 2018 at 11:40 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Jul 18, 2018 at 11:18 AM, Kostya Serebryany <kcc@google.com> wrote:
> > What's ENDBR and do we really need to have it in compiler-rt?
>
> When shadow stack from Intel CET is enabled,  the first instruction of all
> indirect branch targets must be a special instruction, ENDBR.  In this case,

I am confused.
CET is a security mitigation feature (and ENDBR is a pretty weak form of such),
while ASAN is a testing tool, rarely used in production is almost
never as a mitigation (which it is not!).
Why would anyone need to combine CET and ASAN in one process?

Also, CET doesn't exist in the hardware yet, at least not publicly available.
Which means there should be no rush (am I wrong?) and we can do things
in the correct order:
implement the Clang/LLVM support, make the compiler-rt change in LLVM,
merge back to GCC.

--kcc

>
> int res = REAL(swapcontext)(oucp, ucp);
>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^  This function may be
> returned via an indirect branch.
>
> Here compiler must insert ENDBR after call, like
>
> call *bar(%rip)
> endbr64
>
> > As usual, I am opposed to any gcc compiler-rt that bypass upstream.
>
> We want it to be fixed in upstream.  That is why I opened an LLVM bug.
>
>
> > --kcc
> >
> > On Wed, Jul 18, 2018 at 8:37 AM H.J. Lu <hongjiu.lu@intel.com> wrote:
> >>
> >> asan/asan_interceptors.cc has
> >>
> >> ...
> >>   int res = REAL(swapcontext)(oucp, ucp);
> >> ...
> >>
> >> REAL(swapcontext) is a function pointer to swapcontext in libc.  Since
> >> swapcontext may return via indirect branch on x86 when shadow stack is
> >> enabled, we need to call REAL(swapcontext) with indirect_return attribute
> >> on x86 so that compiler can insert ENDBR after REAL(swapcontext) call.
> >>
> >> I opened an LLVM bug:
> >>
> >> https://bugs.llvm.org/show_bug.cgi?id=38207
> >>
> >> But it won't get fixed before indirect_return attribute is added to
> >> LLVM.  I'd like to get it fixed in GCC first.
> >>
> >> Tested on i386 and x86-64.  OK for trunk after
> >>
> >> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01007.html
> >>
> >> is approved?
> >>
> >> Thanks.
> >>
> >>
> >> H.J.
> >> ---
> >>         PR target/86560
> >>         * asan/asan_interceptors.cc (swapcontext): Call REAL(swapcontext)
> >>         with indirect_return attribute on x86.
> >> ---
> >>  libsanitizer/asan/asan_interceptors.cc | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/libsanitizer/asan/asan_interceptors.cc b/libsanitizer/asan/asan_interceptors.cc
> >> index a8f4b72723f..b8dde4f19c5 100644
> >> --- a/libsanitizer/asan/asan_interceptors.cc
> >> +++ b/libsanitizer/asan/asan_interceptors.cc
> >> @@ -267,7 +267,13 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
> >>    uptr stack, ssize;
> >>    ReadContextStack(ucp, &stack, &ssize);
> >>    ClearShadowMemoryForContextStack(stack, ssize);
> >> +#if defined(__x86_64__) || defined(__i386__)
> >> +  int (*real_swapcontext) (struct ucontext_t *, struct ucontext_t *)
> >> +    __attribute__((__indirect_return__)) = REAL(swapcontext);
> >> +  int res = real_swapcontext(oucp, ucp);
> >> +#else
> >>    int res = REAL(swapcontext)(oucp, ucp);
> >> +#endif
> >>    // swapcontext technically does not return, but program may swap context to
> >>    // "oucp" later, that would look as if swapcontext() returned 0.
> >>    // We need to clear shadow for ucp once again, as it may be in arbitrary
> >> --
> >> 2.17.1
> >>
>
>
>
> --
> H.J.