is stdlib/tst-setcontext7 a bad test?
H.J. Lu
hjl.tools@gmail.com
Fri Mar 27 15:42:11 GMT 2020
On Fri, Mar 27, 2020 at 8:36 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 03/27/2020 08:10, H.J. Lu wrote:
> > On Fri, Mar 27, 2020 at 8:01 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 03/27/2020 06:16, H.J. Lu wrote:
> > > > On Fri, Mar 27, 2020 at 5:49 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > >
> > > > > On Fri, Mar 27, 2020 at 5:46 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > >
> > > > > > On Fri, Mar 27, 2020 at 4:04 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > >
> > > > > > > The 03/27/2020 11:10, Florian Weimer wrote:
> > > > > > > > * Szabolcs Nagy:
> > > > > > > > > i'm planing to change swapcontext on aarch64 and i see
> > > > > > > > >
> > > > > > > > > FAIL: stdlib/tst-setcontext7
> > > > > > > > >
> > > > > > > > > that test case seems wrong to me, a simplified version is
> > > > > > > > >
> > > > > > > > > #include <ucontext.h>
> > > > > > > > > #include <stdio.h>
> > > > > > > > >
> > > > > > > > > ucontext_t uc[2];
> > > > > > > > > volatile int count = 0;
> > > > > > > > >
> > > > > > > > > int main ()
> > > > > > > > > {
> > > > > > > > > getcontext (uc+0);
> > > > > > > > > printf ("%d\n", count);
> > > > > > > > > if (count)
> > > > > > > > > setcontext (uc+1);
> > > > > > > > > count++;
> > > > > > > > > swapcontext (uc+1, uc+0);
> > > > > > > > > printf ("done\n");
> > > > > > > > > return 0;
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > this seems to work today and prints
> > > > > > > > >
> > > > > > > > > 0
> > > > > > > > > 1
> > > > > > > > > done
> > > > > > > > >
> > > > > > > > > but i don't think we can guarantee that after a
> > > > > > > > > swapcontext to a getcontext on the same stack frame
> > > > > > > > > resuming the context works: if swapcontext uses the
> > > > > > > > > stack (which is what i plan to do) then that stack
> > > > > > > > > will be corrupted.
> > > > > > > > >
> > > > > > > > > does glibc plan to support this usage?
> > > > > > > >
> > > > > > > > I think it's expected to work. This seems to be one of the more
> > > > > > > > harmless cases.
> > > > > > > >
> > > > > > > > > i.e. should i ensure swapcontext does not use the stack? (and should
> > > > > > > > > gcc ensure no stack is used in the example between getcontext and
> > > > > > > > > swapcontext?)
> > > > > > > >
> > > > > > > > Does the test case pass if you add the returns_twice attribute (or
> > > > > > > > __INDIRECT_RETURN) to getcontext as well (swapcontext already has it)?
> > > > > > >
> > > > > > > yes, the returns_twice attribute on swapcontext
> > > > > > > makes bti work, but that seems ugly (swapcontext
> > > > > > > only returns once).
> > > > > > >
> > > > > > > (getcontext has the return_twice attribute because
> > > > > > > it's a gcc builtin.)
> > > > > > >
> > > > > > > the way i planed to handle swapcontext is to do
> > > > > > >
> > > > > > > swapcontext:
> > > > > > > bti c
> > > > > > > stp x29, x30, [sp, #-16]! // save return address
> > > > > > > bl internal_swapcontext
> > > > > > > bti j
> > > > > > > ldp x29, x30, [sp], #16 // restore return address
> > > > > > > ret
> > > > > > >
> > > > > > > then bti j (landing pad for indirect jump) is
> > > > > > > in libc code, otherwise the compiler has to
> > > > > > > special case swapcontext and emit a bti j after
> > > > > > > every call site (getcontext is already special),
> > > > > > > with my solution swapcontext is not special.
> > > > > > >
> > > > > > > but if glibc plans to support jumping around
> > > > > > > within the same stack frame then my solution
> > > > > > > does not work so i will have to rely on the
> > > > > > > attribute.
> > > > > > >
> > > > > >
> > > > > > Can't you use indirect_return function attribute like the fix for
> > > > > >
> > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85620
> > > > > >
> > > > >
> > > > > You should also check if c-c++-common/asan/swapcontext-test-1.c
> > > > > is OK:
> > > > >
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86560
> > > > >
> > > >
> > > > You also need to provide bits/indirect-return.h for
> > > >
> > > > /* __INDIRECT_RETURN is used on swapcontext to indicate if it requires
> > > > special compiler treatment. */
> > > > #define __INDIRECT_RETURN
> > > >
> > > > See sysdeps/x86/bits/indirect-return.h for example.
> > >
> > > thanks for the heads up.
> > >
> > > i've seen bits/indirec-return.h. i would still prefer
> > > not to special case swapcontext, that can avoid all
> > > sorts of bugs and compatible with existing compilers.
> > >
> > > the context switch to a place on the same stack and
> > > back is what seems unsupportable to me: it requires
> > > the compiler to know the callers of swapcontext and
> > > generate code specially: this is already broken on
> > > all targets without special annotation, since there
> > > is nothing preventing the compiler to use the stack
> > > in ways that breaks such usage.
> > >
> > > if we want to fix this then the attribute has to be
> > > introduced across all targets and gcc has to handle
> > > that in a generic way (i.e. not conditionally adding
> > > the attribute in libc only when cet/bti is enabled)
> > >
> > > but that's a libc api change (changing the type of
> > > swapcontext) affecting users who take the address
> > > of swapcontext.
> >
> > We haven't run into any problems so far.
> >
> > > it's not clear to me if going the x86 way is more
> > > future proof or just no longer supporting usage
> > > like stdlib/tst-setcontext7.
> > >
> >
> > Please take a closer look at these GCC bug reports.
> > You will also see
> >
> > FAIL: c-c++-common/asan/swapcontext-test-1.c
> >
> > if swapcontext isn't properly handled.
>
> i dont think that test would be a problem if the
> BTI J (or ENDBR) landing pad was inside libc in a
> wrapper around the internal swapcontext where
> context switching calls can return to.
lcommit c83b4b824214039fea696083e6a888aa7c9063ce
Author: H.J. Lu <hongjiu.lu@intel.com>
Date: Thu Jul 26 14:48:55 2018 +0000
libsanitizer: Mark REAL(swapcontext) with indirect_return attribute on x86
Cherry-pick compiler-rt revision 337603:
When shadow stack from Intel CET is enabled, the first instruction of all
indirect branch targets must be a special instruction, ENDBR.
lib/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, as 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
I opened an LLVM bug:
https://bugs.llvm.org/show_bug.cgi?id=38207
to add the indirect_return attribute so that it can be used to inform
compiler to insert ENDBR after REAL(swapcontext) call. We mark
REAL(swapcontext) with the indirect_return attribute if it is available.
This fixed:
https://bugs.llvm.org/show_bug.cgi?id=38249
Reviewed By: eugenis
Differential Revision: https://reviews.llvm.org/D49608
PR target/86560
* asan/asan_interceptors.cc (swapcontext) Cherry-pick
compiler-rt revision 337603.
* sanitizer_common/sanitizer_internal_defs.h (__has_attribute):
Likewise.
> the only problem is that currently swapcontext is
> not allowed to use the stack which to me seems like
> an unusual restriction since the compiler can make
> transformations at the call site that adds stack
> usage around the call so it does not seem to be
> worth supporting.
--
H.J.
More information about the Libc-alpha
mailing list