On Wed, Jul 18, 2018 at 12:29 PM H.J. Lu <hjl.to...@gmail.com> wrote: > > On Wed, Jul 18, 2018 at 11:45 AM, Kostya Serebryany <k...@google.com> wrote: > > On Wed, Jul 18, 2018 at 11:40 AM H.J. Lu <hjl.to...@gmail.com> wrote: > >> > >> On Wed, Jul 18, 2018 at 11:18 AM, Kostya Serebryany <k...@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? > > > > CET is transparent to ASAN. It is perfectly OK to use -fcf-protection to > enable CET together with ASAN.
It is ok, but does it make any sense? If anything, the current ASAN's intereceptors are a large blob of security vulnerabilities. If we ever want to use ASAN (or, more likely, HWASAN) as a security mitigation feature, we will need to get rid of these interceptors entirely. > > > 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. > > I am working with our LLVM people to address this. Cool! > > H.J. > > --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...@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. > > > > -- > H.J.
