Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[asan][StackSafety] Custom poisoning is no-op with -asan-use-stack-safety=1 #100640

Open
vitalybuka opened this issue Jul 25, 2024 · 0 comments
Open
Assignees

Comments

@vitalybuka
Copy link
Collaborator

vitalybuka commented Jul 25, 2024

Stack safety does not take into account that we can partially poison alloca.
This may remove mem access instrumentation and not detect bugs detectable with -asan-use-stack-safety=0.

https://godbolt.org/z/ndzbEadM8

#include <sanitizer/asan_interface.h>
#include <iostream>

__attribute__((noinline, weak))
void store(long long& b) {
    __asan_poison_memory_region(&b, sizeof(b));
}

__attribute__((noinline, weak))
long long test() {
    long long b;
    store(b);
    return b;
}

int main() {
    test();
}
@vitalybuka vitalybuka self-assigned this Jul 25, 2024
copybara-service bot pushed a commit to abseil/abseil-cpp that referenced this issue Jul 25, 2024
Many compiler optimizations, even some in Asan, assume that size of
the object on the stack is fixed. Compiler takes into account the lifetime
of the stack object, but it does not expect that size of the object can
change.

This assumption allow to accesses when ever object is alive.

However, this assumption does not work well with custom poisoning
of objects by sanitizers. `Cord` may poison parts of the object
to improve a precision of the bug detection. But compiler still may
load from poisoned parts of the object, assuming it's alive.

So without the patch we can have `false-positive`, because
compiler reordered load from poisoned local before size-check.
llvm/llvm-project#100639

Alternative is to make compiler optimization to know about
possibility of local object resize, but it's not worth of effort,
as it's needed only for sanitizers so far.

Another minor issue fixed with this patch is `false-negative`.
llvm/llvm-project#100640
Asan drops instrumentation of memory accesses proven to be inbound
of local variable. However if we if we take into account `custom
poisoning` we will have to pessimize too often: any random function
call can potentially `resize` a variable by `poisoning`.

We are already using similar `poisoning` workaround in libc++:
https://github.com/llvm/llvm-project/pull/79536/files#diff-534bc2907ddb3b074ded1353d18fd7d578daf1707943b3039bab4ed975aba3b3R772

PiperOrigin-RevId: 656129711
Change-Id: I6d78997da6d31c7ab979a00b84dc9b3b7cffc26f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant