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

install asan libs to out directory so root is not needed #380

Closed
andreya108 opened this issue May 3, 2017 · 28 comments
Closed

install asan libs to out directory so root is not needed #380

andreya108 opened this issue May 3, 2017 · 28 comments
Labels
Milestone

Comments

@andreya108
Copy link

andreya108 commented May 3, 2017

Followed instruction at https://github.com/google/sanitizers/wiki/AddressSanitizerOnAndroid

Encountered problems:

  1. asan_device_setup script does not working. Tried with --use-su on rooted devices:
    a) LG G4s, Android 5.1.1 armeabi-v7a
    b) Nexus 5X, Android 7.1.2 arm64-v8a

  2. linking fails with -fsanitize=address for armeabi-v7a (cannot resolve __android_log_write version "LIBLOG" referenced from libclang_rt.asan-arm-android.so) while it is ok for arm64-v8a

  3. when added manually libclang_rt.asan-aarch64-android.so to project libs got the following crash:

05-03 12:35:58.018 17058-17058/? A/DEBUG: backtrace:
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #00 pc 000000000006bb2c  /system/lib64/libc.so (tgkill+8)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #01 pc 0000000000068f4c  /system/lib64/libc.so (pthread_kill+64)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #02 pc 0000000000023f58  /system/lib64/libc.so (raise+24)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #03 pc 000000000001c810  /system/lib64/libc.so (abort+52)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #04 pc 000000000008529c  /data/app/com.xxxxxxx-2/lib/arm64/libclang_rt.asan-aarch64-android.so
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #05 pc 000000000008a524  /data/app/com.xxxxxxx-2/lib/arm64/libclang_rt.asan-aarch64-android.so
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #06 pc 0000000000077e18  /data/app/com.xxxxxxx-2/lib/arm64/libclang_rt.asan-aarch64-android.so
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #07 pc 000000000008a5a8  /data/app/com.xxxxxxx-2/lib/arm64/libclang_rt.asan-aarch64-android.so (_ZN11__sanitizer11CheckFailedEPKciS1_yy+116)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #08 pc 0000000000062468  /data/app/com.xxxxxxx-2/lib/arm64/libclang_rt.asan-aarch64-android.so
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #09 pc 000000000007747c  /data/app/com.xxxxxxx-2/lib/arm64/libclang_rt.asan-aarch64-android.so
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #10 pc 0000000000366dbc  /data/app/com.xxxxxxx-2/lib/arm64/libxxxxxx.so
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #11 pc 000000000000cac4  /system/bin/linker64 (__dl__ZN6soinfo10call_arrayEPKcPPFvvEmb+360)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #12 pc 000000000000aa88  /system/bin/linker64 (__dl__Z9do_dlopenPKciPK17android_dlextinfoPv+2108)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #13 pc 0000000000007378  /system/bin/linker64 (__dl_dlopen+56)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #14 pc 000000000001376c  /data/app/com.xxxxxxx-2/lib/arm64/libjniproxy.so (Java_xxxxxxxxxx+124)
05-03 12:35:58.019 17058-17058/? A/DEBUG:     #15 pc 00000000009d3fbc  /data/app/com.xxxxxxx-2/oat/arm64/base.odex (offset 0x910000)

How to get asan log for that?


Environment Details

NDK Version: r14b
Build sytem: ndk-build
Host OS: Ubuntu 16.04
Compiler: Clang -std=c++14
ABI: arm64-v8a, armeabi-v6a
STL: gnustl
NDK API level: 19,21
Device API level: 25

@DanAlbert
Copy link
Member

@dimitry-

I don't understand how 2 happened. The library isn't versioned on the device, so the loader should just pick up the unversioned one.

@andreya108
Copy link
Author

Actually android-ndk-r14b/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/3.8.275480/lib/linux/libclang_rt.asan-arm-android.so
contains
"__android_log_write.LIBLOG.liblog.so"
while aarch64 version only "liblog.so" without "__android_log_write.LIBLOG".
I do not know if it will help, but it seems suspicious ...

@DanAlbert
Copy link
Member

$ readelf -sW android-ndk-r14b/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/3.8.275480/lib/linux/libclang_rt.asan-arm-android.so | grep __android_log_write
    29: 00000000     0 FUNC    GLOBAL DEFAULT  UND __android_log_write@LIBLOG (4)
 26654: 00000000     0 FUNC    GLOBAL DEFAULT  UND __android_log_write

That's what I'd expect, so I'm not sure why it doesn't work. I'm going to have to take a closer look, but that may not happen this week.

@stephenhines
Copy link
Collaborator

// 64-bit Android targets don't provide the deprecated __android_log_write.
// Starting with the L release, syslog() works and is preferable to
// __android_log_write.

That comes directly from the sources in compiler-rt (which is where the asan libraries are built), so this mismatch is expected.

@andreya108
Copy link
Author

So, is it impossible to use ASAN for code targeted for android 4.4? Only L+?

@DanAlbert
Copy link
Member

It's supposed to work with KitKat too.

@andreya108
Copy link
Author

Then, what should I use for logging from native code if __android_log_write makes problem and syslog is only from L?

@DanAlbert
Copy link
Member

__android_log_write isn't deprecated. I have no idea why that comment is in compiler-rt. It shouldn't be causing this problem. @dimitry- ?

@JessHolle
Copy link

How did you work around issue (1)?

asan_device_setup does not work at all on Windows, except perhaps Windows 10, as the shell script does not work in Cygwin.

I've been trying to get it to work from a Mac, but ran into issues there as well. The 'adb root' route didn't work. Using --use-su didn't work until I reworked the mount command to workaround argument ordering isssues with mount on Android N. After getting past this, however, I run into still more issues. Is there any reason to expect this script to work with Android N? Has it been tested/verified in recent history?

Or can I just temporarily add the ASAN library to my project and build that way? If so, that's not the documentation should really posit that as an option.

Right now it seems like AddressSanitizer is simply an exercise in frustration.

@DanAlbert
Copy link
Member

Is there any reason to expect this script to work with Android N? Has it been tested/verified in recent history?

We have a test for ASAN and it runs many times a day. We use adb root rather than --use-su though. One thing I notice looking at the script is that the script doesn't do adb disable-verity, which is required on newer devices (since M?). Without this, you can't actually modify the system partition even if it is read/write. @eugenis: any idea if that's intentional, or just something that never got updated.

What device are you using? I test with a Pixel.

@JessHolle
Copy link

I am trying this on a (rooted, obviously) Nexus 9. I never figured out how to get 'adb root' to work -- else I'd have used it.

@DanAlbert
Copy link
Member

Using adb root directly requires a userdebug system image. You can't adb root a user device (see "Build Types" on https://source.android.com/source/building).

Either way, I don't think --use-su is the problem, it's probably verity like I said. Try running adb disable-verity before remounting the file system (I have no idea how this works on a user device).

@andreya108
Copy link
Author

We're tested on Nexus 5x, rooted.
There is no much sense to test on userdebug aosp builds. There are many devices without aosp available, but possible to root.

@andreya108
Copy link
Author

Definitely m+

@JessHolle
Copy link

"adb disable-verity" returns:
disable-verity only works for userdebug builds

This is for a rooted Nexus 9 tablet running Android 7.1.1. (I'm afraid to update it further as it is my understanding that this may undo the rooting -- and that was painful enough to do once...)

Having to root the device to troubleshoot native memory handling is bad enough -- as it seems like there should be hooks already in place that an app can simply opt into. Not being able to get ASAN working with a rooted device is another matter.

As noted above "asan_device_setup --use-su" fails rather quickly as it stands, the remount parameters have to reordered to work around issues in Android 7.x (rw,remount vs. remount,rw). After this, I get further issues -- hence my question about whether this has been tested.

@DanAlbert
Copy link
Member

tbqh I'm not sure why ASAN requires root. @eugenis is the one that did all the ASAN support for Android, so hopefully he'll be able to shed some light on everything above.

@eugenis
Copy link
Collaborator

eugenis commented Jun 7, 2017 via email

@DanAlbert
Copy link
Member

So root is only required if you want to use ASAN in the zygote, right? It's not needed for C++ executables? Since unit tests seem like the most useful place to use ASAN, we should just make that path easier.

@eugenis
Copy link
Collaborator

eugenis commented Jun 7, 2017 via email

@JessHolle
Copy link

In my case I really want to do broad sanity checks of the C++ within an app in various overall use cases, not unit tests of the C++ by itself.

Similarly, ideally the debug malloc library really shouldn't require root. Clearly the APK should have to give permission for or request this sort of meddling, but that's about it.

@DanAlbert
Copy link
Member

In my case I really want to do broad sanity checks of the C++ within an app in various overall use cases, not unit tests of the C++ by itself.

This is the part that currently requires root, as implemented. @eugenis: sounds like you think this could be changed if the work got done on the platform side? Should we get a bug filed for that?

Similarly, ideally the debug malloc library really shouldn't require root.

This is something we're working on, FWIW. @enh: did this get finished?

@andreya108
Copy link
Author

andreya108 commented Jun 29, 2017

Will --use-su ASAN device setup option be fixed in upcoming release or maybe some other workarounds are available? Except using engeneering AOSP build...

The goal is to detect where heap corruption in shared library.

@DanAlbert DanAlbert added this to the r16 milestone Jul 10, 2017
@DanAlbert
Copy link
Member

I'll try to find some time, but this is lower priority than libc++ or Windows LTO.

@enh
Copy link
Contributor

enh commented Jul 10, 2017

@eugenis

In theory, an apk could bring asan runtime library with it, and tell
android that it is OK with being debugged. In that case, android could
apply (2.) for this app only. This feels secure, though I'm not an expert.
This is not implemented.

depends what you mean by "not implemented". in O, there is platform support for doing this: if you have a wrap.sh script in the apk and have debuggable="true" it basically does the "wrap." thing but runs your script rather than starting the child directly. yabinc and agampe know the most about it, so they'll be able to help you out.

@cferris1000 is best placed to comment on the status of "ideally the debug malloc library really shouldn't require root".

@eugenis
Copy link
Collaborator

eugenis commented Jul 11, 2017 via email

@DanAlbert DanAlbert added the clang label Aug 4, 2017
@DanAlbert DanAlbert modified the milestones: r16, r17 Oct 3, 2017
@DanAlbert
Copy link
Member

Unfortunately Android hasn't finished with the Clang update yet, and this has been holding r16 up long enough. This is going to have to wait for r17.

@gpx1000
Copy link

gpx1000 commented Oct 3, 2017

Not that want to plug my own post; but for anyone looking to use ASan in an O production device, one can do that with currently released NDK. Note that it does require su in order to use it; however, have been told that the root requirement will go away in O MR1.
https://virtualrealitypop.com/oreo-ndk-secrets-7d075a9b084
It depends upon the wrap.sh method mentioned by @enh above and example project is linked to within that post.
Looking forward to the clang update in r17!

@DanAlbert DanAlbert changed the title AddressSanitizer is not working in r14b install asan libs to out directory so root is not needed Dec 6, 2017
@DanAlbert
Copy link
Member

Apparently I'd forgotten that I'd filed #540 when I renamed this in December. I've fixed #540 for r17, so the core of this bug is now obsolete (asan_device_setup is no longer needed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants