Skip to content

PR for llvm/llvm-project#58633#420

Merged
tstellar merged 3 commits intorelease/16.xfrom
llvm-issue58633
Apr 19, 2023
Merged

PR for llvm/llvm-project#58633#420
tstellar merged 3 commits intorelease/16.xfrom
llvm-issue58633

Conversation

@llvmbot
Copy link
Copy Markdown
Member

@llvmbot llvmbot commented Apr 16, 2023

vitalybuka and others added 3 commits April 16, 2023 23:43
FileCheck is not very useful here.

(cherry picked from commit fd2cafb)
(cherry picked from commit 3248ca0)
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 llvm/llvm-project#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 llvm/llvm-project#58633

Reviewed By: vitalybuka

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

(cherry picked from commit b380e8b)
@llvmbot
Copy link
Copy Markdown
Member Author

llvmbot commented Apr 16, 2023

@vitalybuka What do you think about merging this PR to the release branch?

@vitalybuka
Copy link
Copy Markdown
Contributor

Let's merge it.

@tstellar tstellar merged commit dbcd2e9 into release/16.x Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASAN reports 'stack-use-after-scope' at seemingly random places after switching to clang15

4 participants