Skip to content

Commit

Permalink
[runtimes][asan] Fix swapcontext interception
Browse files Browse the repository at this point in the history
Resetting oucp's stack to zero in swapcontext interception is incorrect,
since it breaks ucp cleanup after swapcontext returns in some cases:

Say we have two contexts, A and B, and we swapcontext from A to B, do
some work on Bs stack and then swapcontext back from B to A. At this
point shadow memory of Bs stack is in arbitrary state, but since we
can't know whether B will ever swapcontext-ed to again we clean up it's
shadow memory, because otherwise it remains poisoned and blows in
completely unrelated places when heap-allocated memory of Bs context
gets reused later (see #58633
for example). swapcontext prototype is swapcontext(ucontext* oucp,
ucontext* ucp), so in this example A is oucp and B is ucp, and i refer
to the process of cleaning up Bs shadow memory as ucp cleanup.

About how it breaks:
Take the same example with A and B: when we swapcontext back from B to A
the oucp parameter of swapcontext is actually B, and current trunk
resets its stack in a way that it becomes "uncleanupable" later. It
works fine if we do A->B->A, but if we do A->B->A->B->A no cleanup is
performed for Bs stack after B "returns" to A second time. That's
exactly what happens in the test i provided, and it's actually a pretty
common real world scenario.

Instead of resetting oucp's we make use of uc_stack.ss_flags to mark
context as "cleanup-able" by storing stack specific hash. It should be
safe since this field is not used in [get|make|swap]context functions
and is hopefully never meaningfully used in real-world scenarios (and i
haven't seen any).

Fixes #58633

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D137654

(cherry picked from commit b380e8b)
  • Loading branch information
itrofimow authored and tstellar committed Apr 19, 2023
1 parent ec006fb commit dbcd2e9
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 39 deletions.
47 changes: 34 additions & 13 deletions compiler-rt/lib/asan/asan_interceptors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,36 @@ static void ClearShadowMemoryForContextStack(uptr stack, uptr ssize) {
PoisonShadow(bottom, ssize, 0);
}

INTERCEPTOR(int, getcontext, struct ucontext_t *ucp) {
// API does not requires to have ucp clean, and sets only part of fields. We
// use ucp->uc_stack to unpoison new stack. We prefer to have zeroes then
// uninitialized bytes.
ResetContextStack(ucp);
return REAL(getcontext)(ucp);
INTERCEPTOR(void, makecontext, struct ucontext_t *ucp, void (*func)(), int argc,
...) {
va_list ap;
uptr args[64];
// We don't know a better way to forward ... into REAL function. We can
// increase args size if neccecary.
CHECK_LE(argc, ARRAY_SIZE(args));
internal_memset(args, 0, sizeof(args));
va_start(ap, argc);
for (int i = 0; i < argc; ++i) args[i] = va_arg(ap, uptr);
va_end(ap);

# define ENUMERATE_ARRAY_4(start) \
args[start], args[start + 1], args[start + 2], args[start + 3]
# define ENUMERATE_ARRAY_16(start) \
ENUMERATE_ARRAY_4(start), ENUMERATE_ARRAY_4(start + 4), \
ENUMERATE_ARRAY_4(start + 8), ENUMERATE_ARRAY_4(start + 12)
# define ENUMERATE_ARRAY_64() \
ENUMERATE_ARRAY_16(0), ENUMERATE_ARRAY_16(16), ENUMERATE_ARRAY_16(32), \
ENUMERATE_ARRAY_16(48)

REAL(makecontext)
((struct ucontext_t *)ucp, func, argc, ENUMERATE_ARRAY_64());

# undef ENUMERATE_ARRAY_4
# undef ENUMERATE_ARRAY_16
# undef ENUMERATE_ARRAY_64

// Sign the stack so we can identify it for unpoisoning.
SignContextStack(ucp);
}

INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
Expand All @@ -279,9 +303,6 @@ INTERCEPTOR(int, swapcontext, struct ucontext_t *oucp,
ReadContextStack(ucp, &stack, &ssize);
ClearShadowMemoryForContextStack(stack, ssize);

// See getcontext interceptor.
ResetContextStack(oucp);

# if __has_attribute(__indirect_return__) && \
(defined(__x86_64__) || defined(__i386__))
int (*real_swapcontext)(struct ucontext_t *, struct ucontext_t *)
Expand Down Expand Up @@ -658,11 +679,11 @@ void InitializeAsanInterceptors() {
// Intecept jump-related functions.
ASAN_INTERCEPT_FUNC(longjmp);

#if ASAN_INTERCEPT_SWAPCONTEXT
ASAN_INTERCEPT_FUNC(getcontext);
# if ASAN_INTERCEPT_SWAPCONTEXT
ASAN_INTERCEPT_FUNC(swapcontext);
#endif
#if ASAN_INTERCEPT__LONGJMP
ASAN_INTERCEPT_FUNC(makecontext);
# endif
# if ASAN_INTERCEPT__LONGJMP
ASAN_INTERCEPT_FUNC(_longjmp);
#endif
#if ASAN_INTERCEPT___LONGJMP_CHK
Expand Down
2 changes: 1 addition & 1 deletion compiler-rt/lib/asan/asan_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ void AsanApplyToGlobals(globals_op_fptr op, const void *needle);

void AsanOnDeadlySignal(int, void *siginfo, void *context);

void SignContextStack(void *context);
void ReadContextStack(void *context, uptr *stack, uptr *ssize);
void ResetContextStack(void *context);
void StopInitOrderChecking();

// Wrapper for TLS/TSD.
Expand Down
30 changes: 22 additions & 8 deletions compiler-rt/lib/asan/asan_linux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
# include "asan_thread.h"
# include "sanitizer_common/sanitizer_flags.h"
# include "sanitizer_common/sanitizer_freebsd.h"
# include "sanitizer_common/sanitizer_hash.h"
# include "sanitizer_common/sanitizer_libc.h"
# include "sanitizer_common/sanitizer_procmaps.h"

Expand Down Expand Up @@ -211,16 +212,29 @@ void AsanCheckIncompatibleRT() {
# endif // SANITIZER_ANDROID

# if ASAN_INTERCEPT_SWAPCONTEXT
void ReadContextStack(void *context, uptr *stack, uptr *ssize) {
ucontext_t *ucp = (ucontext_t *)context;
*stack = (uptr)ucp->uc_stack.ss_sp;
*ssize = ucp->uc_stack.ss_size;
constexpr u32 kAsanContextStackFlagsMagic = 0x51260eea;

static int HashContextStack(const ucontext_t &ucp) {
MurMur2Hash64Builder hash(kAsanContextStackFlagsMagic);
hash.add(reinterpret_cast<uptr>(ucp.uc_stack.ss_sp));
hash.add(ucp.uc_stack.ss_size);
return static_cast<int>(hash.get());
}

void SignContextStack(void *context) {
ucontext_t *ucp = reinterpret_cast<ucontext_t *>(context);
ucp->uc_stack.ss_flags = HashContextStack(*ucp);
}

void ResetContextStack(void *context) {
ucontext_t *ucp = (ucontext_t *)context;
ucp->uc_stack.ss_sp = nullptr;
ucp->uc_stack.ss_size = 0;
void ReadContextStack(void *context, uptr *stack, uptr *ssize) {
const ucontext_t *ucp = reinterpret_cast<const ucontext_t *>(context);
if (HashContextStack(*ucp) == ucp->uc_stack.ss_flags) {
*stack = reinterpret_cast<uptr>(ucp->uc_stack.ss_sp);
*ssize = ucp->uc_stack.ss_size;
return;
}
*stack = 0;
*ssize = 0;
}
# endif // ASAN_INTERCEPT_SWAPCONTEXT

Expand Down
72 changes: 55 additions & 17 deletions compiler-rt/test/asan/TestCases/Linux/swapcontext_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,23 @@ ucontext_t child_context;

const int kStackSize = 1 << 20;

__attribute__((noinline))
void Throw() {
throw 1;
}
__attribute__((noinline)) void Throw() { throw 1; }

__attribute__((noinline))
void ThrowAndCatch() {
__attribute__((noinline)) void ThrowAndCatch() {
try {
Throw();
} catch(int a) {
} catch (int a) {
printf("ThrowAndCatch: %d\n", a);
}
}

void Child(int mode) {
assert(orig_context.uc_stack.ss_size == 0);
char x[32] = {0}; // Stack gets poisoned.
printf("Child: %p\n", x);
ThrowAndCatch(); // Simulate __asan_handle_no_return().
void Child(int mode, int a, int b, int c) {
char x[32] = {0}; // Stack gets poisoned.
printf("Child: %d\n", x);
assert(a == 'a');
assert(b == 'b');
assert(c == 'c');
ThrowAndCatch(); // Simulate __asan_handle_no_return().
// (a) Do nothing, just return to parent function.
// (b) Jump into the original function. Stack remains poisoned unless we do
// something.
Expand All @@ -53,16 +51,13 @@ void Child(int mode) {
int Run(int arg, int mode, char *child_stack) {
printf("Child stack: %p\n", child_stack);
// Setup child context.
memset(&child_context, 0xff, sizeof(child_context));
getcontext(&child_context);
assert(child_context.uc_stack.ss_size == 0);
child_context.uc_stack.ss_sp = child_stack;
child_context.uc_stack.ss_size = kStackSize / 2;
if (mode == 0) {
child_context.uc_link = &orig_context;
}
makecontext(&child_context, (void (*)())Child, 1, mode);
memset(&orig_context, 0xff, sizeof(orig_context));
makecontext(&child_context, (void (*)())Child, 4, mode, 'a', 'b', 'c');
if (swapcontext(&orig_context, &child_context) < 0) {
perror("swapcontext");
return 0;
Expand All @@ -74,6 +69,47 @@ int Run(int arg, int mode, char *child_stack) {
return child_stack[arg];
}

ucontext_t poll_context;
ucontext_t poller_context;

void Poll() {
swapcontext(&poll_context, &poller_context);

{
char x = 0;
printf("POLL: %p\n", &x);
}

swapcontext(&poll_context, &poller_context);
}

void DoRunPoll(char *poll_stack) {
getcontext(&poll_context);
poll_context.uc_stack.ss_sp = poll_stack;
poll_context.uc_stack.ss_size = kStackSize / 2;
makecontext(&poll_context, Poll, 0);

getcontext(&poller_context);

swapcontext(&poller_context, &poll_context);
swapcontext(&poller_context, &poll_context);

// Touch poll's stack to make sure it's unpoisoned.
for (int i = 0; i < kStackSize; i++) {
poll_stack[i] = i;
}
}

void RunPoll() {
char *poll_stack = new char[kStackSize];

for (size_t i = 0; i < 2; ++i) {
DoRunPoll(poll_stack);
}

delete[] poll_stack;
}

int main(int argc, char **argv) {
char stack[kStackSize + 1];
int ret = 0;
Expand All @@ -82,6 +118,8 @@ int main(int argc, char **argv) {
char *heap = new char[kStackSize + 1];
ret += Run(argc - 1, 0, heap);
ret += Run(argc - 1, 1, heap);
delete [] heap;

RunPoll();
delete[] heap;
return ret;
}

0 comments on commit dbcd2e9

Please sign in to comment.