search

LSPosed / LSPlant

A hook framework for Android Runtime (ART)
https://lsposed.org/LSPlant/
GNU Lesser General Public License v3.0
814 stars 203 forks source link

Fix RestoreBackup call in class_linker #76

Closed xWTF closed 5 months ago

xWTF commented 5 months ago

Problem

In class_linker.hpp, the call from FixupStaticTrampolines(Thread* self, ObjPtr<mirror::Class> klass) to RestoreBackup passes both mirror_class->GetClassDef() and self parameter.

Tracing down to the mirror::Class::PopBackup function, it's obvious that when class_def presents, self parameter is not used:

https://github.com/LSPosed/LSPlant/blob/2cccfae8c1ecaee5592c7a89b5ac1c9692fe76c3/lsplant/src/main/jni/art/mirror/class.hpp#L160-L182

Fix

I just replaced mirror_class->GetClassDef() with nullptr, please correct me if it violates the desired behaviour.

Sidenote

The call to GetClassDef is causing a mysterious SIGSEGV in my AOSP 14 R29 x86_64 emulator (GrapheneOS 2024032100), the exact cause is still unknown after many hours of debugging, therefore I decided to take a look at LSP source and discovered this.

Removing the call to restore all backed up methods in current thread appears to be a practical way to avoid the crash and everything else worked perfectly. Any help in troubleshooting this SIGSEGV is appreciated. Here's the crash log snippet I just reproduced on a fresh build of GrapheneOS 2024040300 emulator:

04-04 16:33:27.209  3064  3064 D LSPosed : magisk_loader.cpp:217#void lspd::MagiskLoader::OnNativeForkAndSpecializePost(JNIEnv *, jstring, jstring): Done prepare
04-04 16:33:27.210  3064  3064 V LSPlant : class_linker.hpp:112#static void lsplant::art::ClassLinker::(anonymous struct)::replace(ClassLinker *, art::Thread *, ObjPtr<mirror::Class>): mirror_class pointer is 0x180272262c212
04-04 16:33:27.211  3064  3064 F libc    : Fatal signal 11 (SIGSEGV), code 128 (SI_KERNEL), fault addr 0x0 in tid 3064 (m.android.shell), pid 3064 (m.android.shell)
04-04 16:33:27.254  3079  3079 I crash_dump64: obtaining output fd from tombstoned, type: kDebuggerdTombstoneProto
04-04 16:33:27.257  3079  3079 I crash_dump64: performing dump of process 3064 (target tid = 3064)
04-04 16:33:27.261  3079  3079 E DEBUG   : unreasonable large fdsan overflow table size 262143, bailing out
04-04 16:33:27.263  3079  3079 E DEBUG   : failed to read process info: failed to open /proc/3064: No such file or directory
04-04 16:33:27.512  3079  3079 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
04-04 16:33:27.512  3079  3079 F DEBUG   : Build fingerprint: 'Android/sdk_phone64_x86_64/emu64x:14/AP1A.240405.002/2024040400:eng/test-keys'
04-04 16:33:27.512  3079  3079 F DEBUG   : Revision: '0'
04-04 16:33:27.512  3079  3079 F DEBUG   : ABI: 'x86_64'
04-04 16:33:27.512  3079  3079 F DEBUG   : Timestamp: 2024-04-04 16:33:27.262065141+0800
04-04 16:33:27.512  3079  3079 F DEBUG   : Process uptime: 0s
04-04 16:33:27.513  3079  3079 F DEBUG   : Cmdline: zygote64
04-04 16:33:27.513  3079  3079 F DEBUG   : pid: 3064, tid: 3064, name: m.android.shell  >>> zygote64 <<<
04-04 16:33:27.513  3079  3079 F DEBUG   : uid: 2000
04-04 16:33:27.513  3079  3079 F DEBUG   : signal 11 (SIGSEGV), code 128 (SI_KERNEL), fault addr 0x0000000000000000
04-04 16:33:27.513  3079  3079 F DEBUG   :     rax 0000000000000001  rbx 0000797b063a0800  rcx 0000778abf776580  rdx 0000000000000002
04-04 16:33:27.513  3079  3079 F DEBUG   :     r8  00007ffe4e3c8080  r9  00007ffe4e3c80a0  r10 0000000000009980  r11 0000000000000216
04-04 16:33:27.513  3079  3079 F DEBUG   :     r12 0000797b063a0800  r13 0000000000000000  r14 000180272262c212  r15 0000000000000000
04-04 16:33:27.513  3079  3079 F DEBUG   :     rdi 000180272262c212  rsi 00007ffe4e338880
04-04 16:33:27.513  3079  3079 F DEBUG   :     rbp 0000797b063a0800  rsp 00007ffe4e33a180  rip 0000778abf776581
04-04 16:33:27.513  3079  3079 F DEBUG   : 30 total frames
04-04 16:33:27.513  3079  3079 F DEBUG   : backtrace:
04-04 16:33:27.513  3079  3079 F DEBUG   :       #00 pc 0000000000d76581  /apex/com.android.art/lib64/libartd.so (art::mirror::Class::GetClassDef()+1)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #01 pc 00000000001152aa  [lsposed] (lsplant::art::ClassLinker::'unnamed9'::replace(lsplant::art::ClassLinker*, lsplant::art::Thread*, lsplant::art::ObjPtr<lsplant::art::mirror::Class>)+138)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #02 pc 0000000000907545  /apex/com.android.art/lib64/libartd.so (art::ClassLinker::MarkClassInitialized(art::Thread*, art::Handle<art::mirror::Class>)+133)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #03 pc 000000000094dda4  /apex/com.android.art/lib64/libartd.so (art::ClassLinker::InitializeClass(art::Thread*, art::Handle<art::mirror::Class>, bool, bool)+3636)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #04 pc 0000000000905306  /apex/com.android.art/lib64/libartd.so (art::ClassLinker::EnsureInitialized(art::Thread*, art::Handle<art::mirror::Class>, bool, bool)+214)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #05 pc 0000000000cdc5ce  /apex/com.android.art/lib64/libartd.so (art::EnsureInitialized(art::Thread*, art::ObjPtr<art::mirror::Class>)+254)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #06 pc 0000000000cdbdda  /apex/com.android.art/lib64/libartd.so (art::FindMethodJNI(art::ScopedObjectAccess const&, _jclass*, char const*, char const*, bool)+58)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #07 pc 0000000000ce9a39  /apex/com.android.art/lib64/libartd.so (art::JNI<false>::GetStaticMethodID(_JNIEnv*, _jclass*, char const*, char const*)+169)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #08 pc 0000000000cbe06b  /apex/com.android.art/lib64/libartd.so (art::(anonymous namespace)::CheckJNI::GetMethodIDInternal(char const*, _JNIEnv*, _jclass*, char const*, char const*, bool)+283)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #09 pc 0000000000cb373e  /apex/com.android.art/lib64/libartd.so (art::(anonymous namespace)::CheckJNI::GetStaticMethodID(_JNIEnv*, _jclass*, char const*, char const*)+30)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #10 pc 00000000000fe218  [lsposed] (void lspd::Context::FindAndCall<int, _jstring*&, _jstring*&, lsplant::ScopedLocalRef<_jobject*>&>(_JNIEnv*, std::_LIBCPP_ABI_NAMESPACE::basic_string_view<char, std::_LIBCPP_ABI_NAMESPACE::char_traits<char> >, std::_LIBCPP_ABI_NAMESPACE::basic_string_view<char, std::_LIBCPP_ABI_NAMESPACE::char_traits<char> >, int&&, _jstring*&, _jstring*&, lsplant::ScopedLocalRef<_jobject*>&) const+56)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #11 pc 00000000000fdf4a  [lsposed] (lspd::MagiskLoader::OnNativeForkAndSpecializePost(_JNIEnv*, _jstring*, _jstring*)+1498)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #12 pc 000000000026d195  /system/lib64/libandroid_runtime.so (android::com_android_internal_os_Zygote_nativeForkAndSpecialize(_JNIEnv*, _jclass*, int, int, _jintArray*, int, _jobjectArray*, int, _jstring*, _jstring*, _jintArray*, _jintArray*, unsigned char, _jstring*, _jstring*, unsigned char, _jobjectArray*, _jobjectArray*, unsigned char, unsigned char, unsigned char, _jlongArray*)+1237)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #13 pc 00000000001588c3  /system/framework/x86_64/boot-framework.oat (art_jni_trampoline+531)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #14 pc 00000000004f03ff  /apex/com.android.art/lib64/libartd.so (nterp_helper+2319)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #15 pc 0000000000523a3c  /system/framework/framework.jar (com.android.internal.os.Zygote.forkAndSpecialize+96)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #16 pc 00000000004e1883  /system/framework/x86_64/boot-framework.oat (com.android.internal.os.ZygoteConnection.processCommand+1571)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #17 pc 00000000004e5309  /system/framework/x86_64/boot-framework.oat (com.android.internal.os.ZygoteServer.runSelectLoop+1865)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #18 pc 00000000004e32f7  /system/framework/x86_64/boot-framework.oat (com.android.internal.os.ZygoteInit.main+1559)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #19 pc 00000000004f9636  /apex/com.android.art/lib64/libartd.so (art_quick_invoke_static_stub+806)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #20 pc 00000000008e280d  /apex/com.android.art/lib64/libartd.so (art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*)+1133)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #21 pc 0000000000ed3e4d  /apex/com.android.art/lib64/libartd.so (art::JValue art::InvokeWithVarArgs<art::ArtMethod*>(art::ScopedObjectAccessAlreadyRunnable const&, _jobject*, art::ArtMethod*, __va_list_tag*)+317)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #22 pc 0000000000ed4520  /apex/com.android.art/lib64/libartd.so (art::JValue art::InvokeWithVarArgs<_jmethodID*>(art::ScopedObjectAccessAlreadyRunnable const&, _jobject*, _jmethodID*, __va_list_tag*)+80)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #23 pc 0000000000d1d4bd  /apex/com.android.art/lib64/libartd.so (art::JNI<true>::CallStaticVoidMethodV(_JNIEnv*, _jclass*, _jmethodID*, __va_list_tag*)+125)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #24 pc 0000000000cbe949  /apex/com.android.art/lib64/libartd.so (art::(anonymous namespace)::CheckJNI::CallMethodV(char const*, _JNIEnv*, _jobject*, _jclass*, _jmethodID*, __va_list_tag*, art::Primitive::Type, art::InvokeType)+2073)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #25 pc 0000000000cb444e  /apex/com.android.art/lib64/libartd.so (art::(anonymous namespace)::CheckJNI::CallStaticVoidMethodV(_JNIEnv*, _jclass*, _jmethodID*, __va_list_tag*)+30)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #26 pc 000000000015f848  /system/lib64/libandroid_runtime.so (_JNIEnv::CallStaticVoidMethod(_jclass*, _jmethodID*, ...)+136)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #27 pc 000000000016c230  /system/lib64/libandroid_runtime.so (android::AndroidRuntime::start(char const*, android::Vector<android::String8> const&, bool)+896)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #28 pc 0000000000003089  /system/bin/app_process64 (main+1609)
04-04 16:33:27.513  3079  3079 F DEBUG   :       #29 pc 000000000005aa0f  /apex/com.android.runtime/lib64/bionic/libc.so (__libc_init+95)
yujincheng08 commented 5 months ago

This is not correct. When class_def is not present, this is specially for async entrypoint setting, which is done in batch. When class_def is present, it will only restore backup for that class, but it does not mean all classes in this thread are done setting entrypoint.

If you change it to nullptr, it may cause a race condition.

xWTF commented 5 months ago

I see, thanks for your correction.

Further testing reveals that the same code (with mirror_class->GetClassDef() called) does NOT cause SIGSEGV:

For anyone who comes from search in the future, you can either switch to a userdebug or user build or set PRODUCT_ART_TARGET_INCLUDE_DEBUG_BUILD := false in your mk files to prevent the selection of the debug variant and still get most benefits from eng builds.

I assume there must be something wrong within these ifs, however it's already taking like 10 hours to troubleshoot this segfault, so I'm not planning to go ahead.

image

BTW sorry for the midnight reply, the AOSP build cycles really take a lot of time to do, especially when I switch to another variant which disables the incremental build.