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

["H.J. Lu" <hjl dot tools at gmail dot com>]

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,

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.