-
Notifications
You must be signed in to change notification settings - Fork 258
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
[BUG] seccomp issues with asan on x86_64 API 27 #1298
Comments
We see this error on |
@eugenis any idea? |
Hi @DanAlbert - I'm trying to run Asan on our native library (android), and I'm seeing an error very similar to above. Here's the logcat -
NDK Version: r22b, 23.1.7779620 |
@enh-google any idea? I'm not very familiar with seccomp and the results we've seen here seem completely broken. |
Hi @DanAlbert / @enh-google - I'm facing a similar issue with API level 30 as well. This time, the error is -
Please note that I'm trying running ASAN on x86_64 android emulator on macOS. Hope this is alright. |
We do support it and there definitely appears to be a bug here, but we don't know what it is yet.
#431. I don't know how to do this currently. |
i haven't had time to try to reproduce this yet, but how do we build asan? it looks like
but at the same time, we shouldn't even need that because we explicitly allow the legacy syscalls for exactly this reason:
so something definitely doesn't make sense here... |
Hi @enh-google, sorry was that question intended for me? As I understand asan comes bundled with ndk, so I'm not very sure how it's built. |
hmm... trying to repro locally, i can't run asan at all:
one thing i did notice while following the instructions though is that we ask people to copy the compiler_rt .so file into their project --- so it's quite possible that people are (correctly) claiming to use a current NDK but not realizing that they might have an out-of-date compiler_rt .so file copied into their jniLibs directory? |
ASan shadow was supposed to be located in the [0x00007fff7000-0x10007fff7fff] range. from the maps, it looks like ART's "Sentinel fault page" is the issue?
but that code is old enough that i'm mentioned in the git blame! |
or with x86-64 API 27, i get a null pointer dereference in the linker...
and the same for x86 API 27 too. for the record, this is with thee Library/Android/sdk/ndk/21.2.6472646/toolchains/llvm/prebuilt/darwin-x86_64/lib64/clang/9.0.8/lib/linux/libclang_rt.asan-*-android.so compiler_rt files. why is that the newest NDK on my machine?! |
i get the same results with Library/Android/sdk/ndk/23.1.7779620/toolchains/llvm/prebuilt/darwin-x86_64/lib64/clang/12.0.8/lib/linux/libclang_rt.asan-x86_64-android.so though, so it's not just that... |
and on a real arm64 device i get
so i haven't actually managed to get asan working anywhere yet... |
Thanks @enh-google for the investigation and spending your time on the issue! |
yeah, sorry for the obviously poor state this is currently in! we'll keep looking, but in the meantime i did want to check that you know about gwp-asan (https://developer.android.com/ndk/guides/gwp-asan) which -- for the cost of one line in your AndroidManifest.xml -- you can use in the field to detect memory issues (with crashes showing up in the play developer console [or whatever custom solution you're using] just like normal), or hwasan (https://source.android.com/devices/tech/debug/hwasan) which -- while it involves running a custom OS build and is arm64-only -- is really fast hardware-assisted asan; fast enough that we have people using these builds for dogfooding. |
We've had complaints about ubsan issues for years, but never got to the bottom of them, or saw them ourselves in testing. For some reason (still not understood) we've started to see this ourselves in T and downstream branches. So for now, let's just punch that extra hole. Longer term, ubsan should stop needing any of this, so once https://reviews.llvm.org/D124212 is in an LTS NDK, we should be able to get rid of this _and_ the existing sanitizer-related holes. Bug: android/ndk#1298 Bug: http://b/229989971 Test: treehugger Change-Id: Id42cb29c4e943c0080c0d34ce4e5d6d1b32da9e8
oops, sorry for not updating publicly on this... i've written a lot of words internally but didn't realize until it was pointed out to me by @DanAlbert that i'd said nothing externally. we started seeing this internally, and talked to the folks who developed the sanitizers. it looks like we've been building them in a way that's incompatible with our seccomp filter. for Android T we've basically just added the missing syscall to the seccomp allowlist. but the folks who owned the sanitizers think it's time to retire this code anyway and just always use the new syscalls (such as openat(2) vs open(2)). so although we can't normally fix the past, once we've pulled that upstream change and rebuilt everything, NDKs after that point should fix this because they won't try to use the obsolete system calls. (no ETA on that yet. http://b/229989971 is the internal bug for any googler who wants to check. https://reviews.llvm.org/D124212 is the upstream LLVM change we're waiting for. that link should work for everyone.) |
Will be r26 at the earliest. Not a regression so will not be backported to r25 (I expect it's not a clean cherry-pick, so it would be a risky backport). |
We've had complaints about ubsan issues for years, but never got to the bottom of them, or saw them ourselves in testing. For some reason (still not understood) we've started to see this ourselves in T and downstream branches. So for now, let's just punch that extra hole. Longer term, ubsan should stop needing any of this, so once https://reviews.llvm.org/D124212 is in an LTS NDK, we should be able to get rid of this _and_ the existing sanitizer-related holes. Bug: android/ndk#1298 Bug: http://b/229989971 Test: treehugger Change-Id: Id42cb29c4e943c0080c0d34ce4e5d6d1b32da9e8 Signed-off-by: Akash Srivastava <[email protected]>
We've had complaints about ubsan issues for years, but never got to the bottom of them, or saw them ourselves in testing. For some reason (still not understood) we've started to see this ourselves in T and downstream branches. So for now, let's just punch that extra hole. Longer term, ubsan should stop needing any of this, so once https://reviews.llvm.org/D124212 is in an LTS NDK, we should be able to get rid of this _and_ the existing sanitizer-related holes. Bug: android/ndk#1298 Bug: http://b/229989971 Test: treehugger Change-Id: Id42cb29c4e943c0080c0d34ce4e5d6d1b32da9e8 Signed-off-by: minaripenguin <[email protected]>
We've had complaints about ubsan issues for years, but never got to the bottom of them, or saw them ourselves in testing. For some reason (still not understood) we've started to see this ourselves in T and downstream branches. So for now, let's just punch that extra hole. Longer term, ubsan should stop needing any of this, so once https://reviews.llvm.org/D124212 is in an LTS NDK, we should be able to get rid of this _and_ the existing sanitizer-related holes. Bug: android/ndk#1298 Bug: http://b/229989971 Test: treehugger Change-Id: Id42cb29c4e943c0080c0d34ce4e5d6d1b32da9e8 Signed-off-by: Akash Srivastava <[email protected]>
We've had complaints about ubsan issues for years, but never got to the bottom of them, or saw them ourselves in testing. For some reason (still not understood) we've started to see this ourselves in T and downstream branches. So for now, let's just punch that extra hole. Longer term, ubsan should stop needing any of this, so once https://reviews.llvm.org/D124212 is in an LTS NDK, we should be able to get rid of this _and_ the existing sanitizer-related holes. Bug: android/ndk#1298 Bug: http://b/229989971 Test: treehugger Change-Id: Id42cb29c4e943c0080c0d34ce4e5d6d1b32da9e8 Signed-off-by: Akash Srivastava <[email protected]>
We've had complaints about ubsan issues for years, but never got to the bottom of them, or saw them ourselves in testing. For some reason (still not understood) we've started to see this ourselves in T and downstream branches. So for now, let's just punch that extra hole. Longer term, ubsan should stop needing any of this, so once https://reviews.llvm.org/D124212 is in an LTS NDK, we should be able to get rid of this _and_ the existing sanitizer-related holes. Bug: android/ndk#1298 Bug: http://b/229989971 Test: treehugger Change-Id: Id42cb29c4e943c0080c0d34ce4e5d6d1b32da9e8 (cherry picked from commit a0745ce) Merged-In: Id42cb29c4e943c0080c0d34ce4e5d6d1b32da9e8
Hi @DanAlbert Thank you |
Nothing will be backported to r25. It is not supported. |
We've had complaints about ubsan issues for years, but never got to the bottom of them, or saw them ourselves in testing. For some reason (still not understood) we've started to see this ourselves in T and downstream branches. So for now, let's just punch that extra hole. Longer term, ubsan should stop needing any of this, so once https://reviews.llvm.org/D124212 is in an LTS NDK, we should be able to get rid of this _and_ the existing sanitizer-related holes. Bug: android/ndk#1298 Bug: http://b/229989971 Test: treehugger Change-Id: Id42cb29c4e943c0080c0d34ce4e5d6d1b32da9e8
Description
Isn't syscall 0
read
(https://cs.android.com/android/platform/superproject/+/master:bionic/libc/kernel/uapi/asm-x86/asm/unistd_64.h;l=21;drc=bb9fcb46361ddb55aac7faf639de5088a09b9b8e)? That can't be right.https://github.com/DanAlbert/asan-seccomp-repro repros on the API 27 x86_64 emulator.
Environment Details
The text was updated successfully, but these errors were encountered: