search

abforce / xposed_art_n

ART module for a built-in enabled Xposed firmware based on AOSP 7
Other
171 stars 43 forks source link

xposed hooked methods should be excluded in more places, like native & runtime #16

Closed ahronshor closed 7 years ago

ahronshor commented 7 years ago

The Thread::VisitRoots function goes to the Thread::VisitQuickFrame function, and there are also native and Runtime methods that are excluded like in ArtMethod::VisitRoots therefore, xposed hooked methods should also be excluded.

ahronshor commented 7 years ago

This might be a solution to the problem you referred to in the README: 

While translating rovo89's commits to Andorid N, VisitRoots method of art_method-inl.h had been changed completely, so I was just uncertain whether my port is correct or not. I was just shooting in the dark, to hopefully hit the target :).
aviraxp commented 7 years ago

Can confirm that it fixes many crashes, but there are still many which related to JIT.

abforce commented 7 years ago

Did you test it with XPrivacy enabled? Thanks for contributing anyway.

avently commented 7 years ago

@abforce I build art with this commit and apps had crashed before works now. It is awesome! Now i can use half-working Xposed without crashes. @ahronshor @abforce thank you for that

I tested Xprivacy too. No crashes but no hooks working at all...

abforce commented 7 years ago

Yeah. It seems to work. I'm just feeling I'm walking on air, no stability I can think of. Still lots of testing might be required. Thank you @ahronshor.

By the way, I've before tested it with IsProxyMethod(true), it failed. However surprisingly your commit works. Keep up the good news man!

ahronshor commented 7 years ago

it's clear !IsProxyMethod() excludes proxy and xposed. !IsProxyMethod(true) excludes proxy. but here (!m->IsProxyMethod() || m->IsConstructor()) excludes proxy and xposed constructors only. and (!m->IsProxyMethod(true) || m->IsConstructor()) excludes proxy constructors only.

And therefore !m->IsXposedHookedMethod() should be added

Yangff commented 7 years ago

crash is a good sigh because you can collect some logs and analysing it...

But no hooks work for xprivacy??

abforce commented 7 years ago

@ahronshor Yes, without this, hooked constructors can pass through the if and your commit doesn't allow.

@Yangff I tested. Hooked methods do work.

abforce commented 7 years ago

Unfortunately today my phone fell into a bootloop. Still there's a problem with GC. Not sure it's because of your commit or not.

07-12 22:57:46.312   356   356 W         : debuggerd: handling request: pid=9203 uid=1000 gid=1000 tid=9203
07-12 22:57:46.332  9379  9379 E         : debuggerd: Unable to connect to activity manager (connect failed: Connection refused)
07-12 22:57:46.382  9379  9379 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
07-12 22:57:46.382  9379  9379 F DEBUG   : Build fingerprint: 'Android/aosp_bullhead/bullhead:7.1.1/NMF26V/root07170845:userdebug/test-keys'
07-12 22:57:46.382  9379  9379 F DEBUG   : Revision: 'rev_1.0'
07-12 22:57:46.382  9379  9379 F DEBUG   : ABI: 'arm64'
07-12 22:57:46.382  9379  9379 F DEBUG   : pid: 9203, tid: 9203, name: system_server  >>> system_server <<<
07-12 22:57:46.382  9379  9379 F DEBUG   : signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
07-12 22:57:46.389  9379  9379 F DEBUG   : Abort message: 'art/runtime/gc/collector/mark_sweep.cc:415] Tried to mark 0xa2d1f138 not contained by any spaces'
07-12 22:57:46.389  9379  9379 F DEBUG   :     x0   0000000000000000  x1   00000000000023f3  x2   0000000000000006  x3   0000000000000008
07-12 22:57:46.389  9379  9379 F DEBUG   :     x4   00000000000000fa  x5   0000000000000000  x6   0000007fa7f34000  x7   0000000000000000
07-12 22:57:46.389  9379  9379 F DEBUG   :     x8   0000000000000083  x9   ffffffffffffffdf  x10  0000000000000000  x11  0000000000000001
07-12 22:57:46.389  9379  9379 F DEBUG   :     x12  ffffffffffffe000  x13  ffffffffffffefff  x14  0000000000000000  x15  001299157bee8237
07-12 22:57:46.389  9379  9379 F DEBUG   :     x16  0000007fa5d30ee0  x17  0000007fa5cdaaac  x18  0000000000000000  x19  0000007fa7fe8b40
07-12 22:57:46.389  9379  9379 F DEBUG   :     x20  0000000000000006  x21  0000007fa7fe8a98  x22  000000000000000b  x23  0000000000006bca
07-12 22:57:46.389  9379  9379 F DEBUG   :     x24  0000007fa43fe000  x25  0000007fa434c9ca  x26  0000007fd5836ed1  x27  ffffffffffffffff
07-12 22:57:46.389  9379  9379 F DEBUG   :     x28  0000007fa434c963  x29  0000007fd5836db0  x30  0000007fa5cd7ed8
07-12 22:57:46.389  9379  9379 F DEBUG   :     sp   0000007fd5836d90  pc   0000007fa5cdaab4  pstate 0000000060000000
07-12 22:57:46.394  9379  9379 F DEBUG   : 
07-12 22:57:46.394  9379  9379 F DEBUG   : backtrace:
07-12 22:57:46.394  9379  9379 F DEBUG   :     #00 pc 000000000006bab4  /system/lib64/libc.so (tgkill+8)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #01 pc 0000000000068ed4  /system/lib64/libc.so (pthread_kill+64)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #02 pc 0000000000023f58  /system/lib64/libc.so (raise+24)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #03 pc 000000000001c810  /system/lib64/libc.so (abort+52)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #04 pc 00000000004357e0  /system/lib64/libart.so (_ZN3art7Runtime5AbortEPKc+468)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #05 pc 00000000000e65e8  /system/lib64/libart.so (_ZN3art10LogMessageD2Ev+1612)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #06 pc 000000000046666c  /system/lib64/libart.so (_ZN3artL40UnsafeLogFatalForThreadSuspendAllTimeoutEv+848)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #07 pc 0000000000465f8c  /system/lib64/libart.so (_ZN3art10ThreadList10SuspendAllEPKcb+592)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #08 pc 00000000000e0874  /system/lib64/libart.so (_ZN3art9ArtMethod16EnableXposedHookERNS_18ScopedObjectAccessEP8_jobject+584)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #09 pc 0000000000004edc  /system/lib64/libxposed_art.so (_ZN6xposed29XposedBridge_hookMethodNativeEP7_JNIEnvP7_jclassP8_jobjectS5_iS5_+588)
07-12 22:57:46.394  9379  9379 F DEBUG   :     #10 pc 0000000000072ee4  /system/framework/oat/arm64/XposedBridge.odex (offset 0x5f000)
07-12 22:57:47.756   356   356 W         : debuggerd: resuming target 9203
07-12 22:57:47.762   391   391 I ServiceManager: service 'user.xprivacy481' died
07-12 22:57:47.763   514   514 E         : eof
07-12 22:57:47.763   514   514 E         : failed to read size
07-12 22:57:47.763   514   514 I         : closing connection
07-12 22:57:47.763  8982  8982 I Zygote  : Process 9203 exited due to signal (6)
07-12 22:57:47.763  8982  8982 E Zygote  : Exit zygote because system server (9203) has terminated
aviraxp commented 7 years ago

I had stacktrace like this even before this commit...But that is for hooked apps

Yangff commented 7 years ago

https://github.com/abforce/xposed_art_n/blob/1d14337b858cabd184335804b178f16849186f89/runtime/art_method.cc#L579

@abforce Looks like some problem with the hooking. Timeout when suspending (30s timeout) threads. GC will suspend threads either.. maybe it's related? deadlock?

No.. Not looks like a deadlock... GC marking will lock heap only, not thread list.

Anyway, this should be another issue..

Yangff commented 7 years ago

if it's possible, go check VLOG for thread_list.cc to find out who held the lock of mutator_lock_. while xposed trying to suspendall..

Or just check which thread is runnalbe...