diff --git a/lib/android.js b/lib/android.js index d023166..7aa320f 100644 --- a/lib/android.js +++ b/lib/android.js @@ -1802,67 +1802,98 @@ function makeArtController (api, vm) { const code = ` #include -extern GMutex lock; -extern GHashTable * methods; -extern GHashTable * replacements; extern gpointer last_seen_art_method; +extern gpointer entries; extern gpointer get_oat_quick_method_header_impl (gpointer method, gpointer pc); +/* + * Hooked-method mappings are published as an immutable singly-linked + * list. Hook install/revert always runs on the JS thread (single + * writer), which prepends a fresh entry and republishes the head with a + * plain aligned pointer store; the trampolines on arbitrary ART threads + * walk the list lock-free, so they never block against ART thread + * suspension. Retired lists are deliberately leaked: there is no safe + * reclamation point without suspending all ART threads. + */ +typedef struct _ReplacementEntry ReplacementEntry; + +struct _ReplacementEntry +{ + gpointer original; + gpointer replacement; + ReplacementEntry * next; +}; + +static ReplacementEntry * +replacement_entry_new (gpointer original, gpointer replacement) +{ + ReplacementEntry * entry; + + entry = g_new (ReplacementEntry, 1); + entry->original = original; + entry->replacement = replacement; + entry->next = NULL; + + return entry; +} + void init (void) { - g_mutex_init (&lock); - methods = g_hash_table_new_full (NULL, NULL, NULL, NULL); - replacements = g_hash_table_new_full (NULL, NULL, NULL, NULL); + entries = NULL; } void finalize (void) { - g_hash_table_unref (replacements); - g_hash_table_unref (methods); - g_mutex_clear (&lock); + ReplacementEntry * entry, * next; + + for (entry = entries; entry != NULL; entry = next) + { + next = entry->next; + g_free (entry); + } } gboolean is_replacement_method (gpointer method) { - gboolean is_replacement; - - g_mutex_lock (&lock); + ReplacementEntry * entry; - is_replacement = g_hash_table_contains (replacements, method); - - g_mutex_unlock (&lock); + for (entry = entries; entry != NULL; entry = entry->next) + { + if (entry->replacement == method) + return TRUE; + } - return is_replacement; + return FALSE; } gpointer get_replacement_method (gpointer original_method) { - gpointer replacement_method; - - g_mutex_lock (&lock); + ReplacementEntry * entry; - replacement_method = g_hash_table_lookup (methods, original_method); - - g_mutex_unlock (&lock); + for (entry = entries; entry != NULL; entry = entry->next) + { + if (entry->original == original_method) + return entry->replacement; + } - return replacement_method; + return NULL; } void set_replacement_method (gpointer original_method, gpointer replacement_method) { - g_mutex_lock (&lock); + ReplacementEntry * entry; - g_hash_table_insert (methods, original_method, replacement_method); - g_hash_table_insert (replacements, replacement_method, original_method); + entry = replacement_entry_new (original_method, replacement_method); + entry->next = entries; - g_mutex_unlock (&lock); + entries = entry; } void @@ -1870,55 +1901,54 @@ synchronize_replacement_methods (guint quick_code_offset, void * nterp_entrypoint, void * quick_to_interpreter_bridge) { - GHashTableIter iter; - gpointer hooked_method, replacement_method; - - g_mutex_lock (&lock); + ReplacementEntry * entry; - g_hash_table_iter_init (&iter, methods); - while (g_hash_table_iter_next (&iter, &hooked_method, &replacement_method)) + for (entry = entries; entry != NULL; entry = entry->next) { void ** quick_code; - *((uint32_t *) replacement_method) = *((uint32_t *) hooked_method); + *((uint32_t *) entry->replacement) = *((uint32_t *) entry->original); - quick_code = hooked_method + quick_code_offset; + quick_code = entry->original + quick_code_offset; if (*quick_code == nterp_entrypoint) *quick_code = quick_to_interpreter_bridge; } - - g_mutex_unlock (&lock); } void delete_replacement_method (gpointer original_method) { - gpointer replacement_method; + ReplacementEntry * old_entry, * entry; - g_mutex_lock (&lock); + old_entry = entries; + entries = NULL; - replacement_method = g_hash_table_lookup (methods, original_method); - if (replacement_method != NULL) + while (old_entry != NULL) { - g_hash_table_remove (methods, original_method); - g_hash_table_remove (replacements, replacement_method); - } + if (old_entry->original != original_method) + { + entry = replacement_entry_new (old_entry->original, + old_entry->replacement); + entry->next = entries; + entries = entry; + } - g_mutex_unlock (&lock); + old_entry = old_entry->next; + } } gpointer translate_method (gpointer method) { - gpointer translated_method; - - g_mutex_lock (&lock); + ReplacementEntry * entry; - translated_method = g_hash_table_lookup (replacements, method); - - g_mutex_unlock (&lock); + for (entry = entries; entry != NULL; entry = entry->next) + { + if (entry->replacement == method) + return entry->original; + } - return (translated_method != NULL) ? translated_method : method; + return method; } gpointer @@ -2003,39 +2033,24 @@ on_art_method_pretty_method (GumInvocationContext * ic) void on_leave_gc_concurrent_copying_copying_phase (GumInvocationContext * ic) { - GHashTableIter iter; - gpointer hooked_method, replacement_method; - - g_mutex_lock (&lock); + ReplacementEntry * entry; - g_hash_table_iter_init (&iter, methods); - while (g_hash_table_iter_next (&iter, &hooked_method, &replacement_method)) - *((uint32_t *) replacement_method) = *((uint32_t *) hooked_method); - - g_mutex_unlock (&lock); + for (entry = entries; entry != NULL; entry = entry->next) + *((uint32_t *) entry->replacement) = *((uint32_t *) entry->original); } `; - const lockSize = 8; - const methodsSize = pointerSize; - const replacementsSize = pointerSize; - const lastSeenArtMethodSize = pointerSize; - - const data = Memory.alloc(lockSize + methodsSize + replacementsSize + lastSeenArtMethodSize); + const data = Memory.alloc(pointerSize * 2); - const lock = data; - const methods = lock.add(lockSize); - const replacements = methods.add(methodsSize); - const lastSeenArtMethod = replacements.add(replacementsSize); + const entries = data; + const lastSeenArtMethod = entries.add(pointerSize); const getOatQuickMethodHeaderImpl = api.find((pointerSize === 4) ? '_ZN3art9ArtMethod23GetOatQuickMethodHeaderEj' : '_ZN3art9ArtMethod23GetOatQuickMethodHeaderEm'); const cm = new CModule(code, { - lock, - methods, - replacements, + entries, last_seen_art_method: lastSeenArtMethod, get_oat_quick_method_header_impl: getOatQuickMethodHeaderImpl ?? ptr('0xdeadbeef') }); diff --git a/repro/README.md b/repro/README.md new file mode 100644 index 0000000..7a9305c --- /dev/null +++ b/repro/README.md @@ -0,0 +1,69 @@ +# arm64 dispatch deadlock — reproduction + +Reproduces the process-wide deadlock described in the PR: with +`.implementation` hooks installed, frida-java-bridge's replacement-method +dispatch takes a blocking `GMutex` on ART's hot dispatch paths, which +deadlocks against ART thread suspension (GC, thread flips) on arm64. + +## What you need + +- arm64 Android 13 or 14 emulator (e.g. Cuttlefish, or an arm64 AVD) +- frida >= 17 and frida-server for the same version +- Node.js with `frida-compile` +- Java + `d8` (Android build-tools) — optional, only for the worker dex + +## Steps + +```sh +# 1. Build and push frida-server +adb push frida-server /data/local/tmp/ +adb shell "/data/local/tmp/frida-server &" + +# 2. Disable the JIT (optional but recommended: with the JIT on, the +# workload can instead trip the dispatch-trampoline patch race and +# crash; the deadlock itself fires with the JIT on or off) +adb root +adb shell "setprop dalvik.vm.usejit false && stop && start" + +# 3. Bundle the script against the frida-java-bridge build under test +npm install frida-java-bridge frida-compile # released npm = unfixed +npx frida-compile repro-deadlock-entry.js -o repro-bundled.js +adb push repro-bundled.js /data/local/tmp/ + +# 4. Run it against any app (Settings works fine) +frida -U -f com.android.settings -l /data/local/tmp/repro-bundled.js +``` + +The script hooks frequently-called native methods (forwarding +implementations) and prints a heartbeat every 30 s that includes a Java +round-trip latency probe. + +## Expected results + +**Unfixed bridge (any released frida-java-bridge):** the process wedges +within minutes. The heartbeat stops (the JS thread's next Java call parks +on the dispatch mutex) and the app ANRs. Confirm the signature with +`adb shell kill -3 ` and inspect the newest `/data/anr/anr_*` +(or `debuggerd -b `): several threads blocked in +`pthread_mutex_lock` (bionic `NonPI::MutexLockWithTimeout`) at the SAME +PC in the dispatch trampoline's code, entered from +`art_quick_generic_jni_trampoline`. + +**Fixed bridge (this branch):** no dispatch-mutex cluster appears, the +app's threads stay responsive, and the run completes without a wedge. + +## Notes + +- The deadlock needs concurrent JNI crossing + GC/flip activity. On an + idle app the crossing volume comes from the framework's constant calls + to the hooked time/id methods; if your device is very quiet, bump the + crossing rate by opening the app's settings pages or adding the + optional worker dex (see `ReproWorker.java`). +- Do not drive GCs from the script's own thread: a direct + `Runtime.gc()` (or heavy allocation) from the frida JS thread wedges + that thread against ART's GC-completion wait even with no hooks at + all, which confounds the repro. +- On fast emulators with the JIT enabled the workload can instead trip + the trampoline-prologue patch race (crash) — the deadlock is the + failure mode on slower devices and with the JIT off, which is also + what the ANR-trace signature distinguishes. diff --git a/repro/ReproWorker.java b/repro/ReproWorker.java new file mode 100644 index 0000000..bdc01a9 --- /dev/null +++ b/repro/ReproWorker.java @@ -0,0 +1,38 @@ +/* + * Repro worker: real Java threads that hammer the instrumented JNI + * boundary while driving GC pressure. Loaded via frida's + * Java.openClassFile() from /data/local/tmp/repro-worker.dex (built from + * ReproWorker.java; see README.md). + */ +package com.example.repro; + +public final class ReproWorker implements Runnable { + private static final long RUN_NANOS = 10L * 60L * 1000L * 1000L * 1000L; + private static final int GC_INTERVAL = 4096; + private static final int BATCH = 64; + private static final int SLOT_SIZE = 4096; + + public void run() { + final long end = System.nanoTime() + RUN_NANOS; + final Runtime rt = Runtime.getRuntime(); + final Object marker = new Object(); + final byte[][] slots = new byte[4096][]; + int n = 0; + int counter = 0; + while (System.nanoTime() < end) { + // Every one of these is a hooked native method, so each call + // crosses the patched quick entrypoints (and, on the unfixed + // bridge, the dispatch GMutex). + System.currentTimeMillis(); + System.nanoTime(); + System.identityHashCode(marker); + Thread.currentThread(); + for (int i = 0; i < BATCH; i++) { + slots[n++ % slots.length] = new byte[SLOT_SIZE]; + } + if ((counter++ & (GC_INTERVAL - 1)) == 0) { + rt.gc(); + } + } + } +} diff --git a/repro/repro-deadlock-entry.js b/repro/repro-deadlock-entry.js new file mode 100644 index 0000000..adbbe80 --- /dev/null +++ b/repro/repro-deadlock-entry.js @@ -0,0 +1,12 @@ +/* + * Bundled variant of repro-deadlock.js for testing a specific + * frida-java-bridge build (e.g. this branch's fix). + * + * npm install frida-java-bridge # from this branch, or a local path + * npx frida-compile repro-deadlock-entry.js -o repro-bundled.js + * frida -U -f com.android.settings -l repro-bundled.js + */ +const mod = require('frida-java-bridge'); +const Java = mod.default ?? mod; +globalThis.Java = Java; +require('./repro-deadlock.js'); diff --git a/repro/repro-deadlock.js b/repro/repro-deadlock.js new file mode 100644 index 0000000..0017196 --- /dev/null +++ b/repro/repro-deadlock.js @@ -0,0 +1,164 @@ +'use strict'; + +/* + * Reproduction for: process-wide deadlock on arm64 when .implementation + * hooks are installed (frida-java-bridge replacement-method dispatch + * takes a blocking GMutex on ART's hot dispatch paths). + * + * How it works: + * - hooks frequently-called native methods with forwarding + * implementations. Every call to a hooked method crosses the patched + * quick entrypoints on the CALLING ART thread (the app's binder + * pool, main thread, and background threads call these constantly), + * which is the path that takes the dispatch GMutex on the unfixed + * bridge; + * - prints a heartbeat every 30 s that includes a Java round-trip + * latency probe. If Java wedges, the probe blocks forever and the + * heartbeats stop: that is the bug. + * + * Usage (arm64 Android 13/14 emulator or device, frida >= 17): + * + * # disable the JIT (recommended: with the JIT on, the workload can + * # instead trip the entrypoint patch race and crash; the deadlock + * # fires with the JIT on or off) + * adb root && adb shell "setprop dalvik.vm.usejit false && stop && start" + * + * # build the script bundle (this file is not runnable standalone: + * # the Java bridge must be bundled with it) + * npm install frida-java-bridge frida-compile + * npx frida-compile repro/repro-deadlock-entry.js -o repro-bundled.js + * adb push frida-server /data/local/tmp/ && adb shell \ + * "/data/local/tmp/frida-server &" + * frida -U -f com.android.settings -l repro-bundled.js + * + * Optional: set ALLOC=1 in the script to also drive GC / flip pressure + * from the JS thread (allocation only — never Runtime.gc(), a direct GC + * request from the JS thread wedges even without hooks). The deadlock + * also fires from the framework's own traffic alone, just more slowly. + * + * To reproduce with the UNFIXED bridge, install the released + * frida-java-bridge from npm; to verify the fix, install it from this + * branch. Everything else stays the same. + * + * Expected behaviour: + * - UNFIXED bridge (any released frida-java-bridge): heartbeats stop + * within a few minutes and the app ANRs. Confirm with + * `adb shell kill -3 ` and inspect the newest /data/anr/anr_*: + * several threads blocked in pthread_mutex_lock (bionic + * NonPI::MutexLockWithTimeout) at the SAME PC in jit-cache/anonymous + * code, entered from art_quick_generic_jni_trampoline. + * - FIXED bridge: heartbeats continue for the full run (default + * 10 minutes) and no ANR is produced. + */ + +const HOOKS = [ + ['java.lang.System', 'currentTimeMillis', []], + ['java.lang.System', 'nanoTime', []], + ['java.lang.System', 'identityHashCode', ['java.lang.Object']], + ['java.lang.Thread', 'currentThread', []], + ['android.os.SystemClock', 'uptimeMillis', []], + ['android.os.SystemClock', 'elapsedRealtime', []], + ['android.os.Process', 'myTid', []], + ['java.lang.Runtime', 'maxMemory', []], +]; + +const RUN_MILLIS = 10 * 60 * 1000; // 10 minutes +const JAVA_STALL_MILLIS = 5000; // heartbeat probe threshold + +let startedAt = 0; + +function installHooks() { + const installed = []; + for (const [clsName, methodName, overloadArgs] of HOOKS) { + try { + const cls = Java.use(clsName); + const method = overloadArgs.length > 0 + ? cls[methodName].overload(...overloadArgs) + : cls[methodName].overload(); + method.implementation = function (...args) { + return this[methodName](...args); + }; + installed.push(clsName + '.' + methodName); + } catch (e) { + console.log('[repro] skip ' + clsName + '.' + methodName + ': ' + e); + } + } + console.log('[repro] hooked ' + installed.length + '/' + HOOKS.length + + ' native methods'); +} + +function startAllocator() { + // Allocation pressure only: triggers the GC daemon's normal cycles and + // thread flips. Deliberately NOT Runtime.gc(): a direct GC request from + // the JS thread blocks in ART's GC-completion wait in Runnable state + // and wedges even without any hooks, which would confound the repro. + const allocs = []; + setInterval(() => { + allocs.push(Java.array('byte', new Array(16384).fill(0))); + if (allocs.length > 128) { + allocs.length = 0; + } + }, 200); + console.log('[repro] allocator running (GC / flip pressure)'); +} + +function startHeartbeat() { + const System = Java.use('java.lang.System'); + const timer = setInterval(() => { + const t0 = Date.now(); + let roundTrip; + try { + System.currentTimeMillis(); + roundTrip = Date.now() - t0; + } catch (e) { + console.log('[repro] heartbeat java probe failed: ' + e); + roundTrip = -1; + } + const elapsed = Date.now() - startedAt; + console.log('[repro] alive after ' + elapsed + ' ms (java round-trip ' + + roundTrip + ' ms)'); + if (roundTrip > JAVA_STALL_MILLIS) { + console.log('[repro] JAVA STALLED: dispatch wedged (bug reproduced)'); + clearInterval(timer); + } else if (elapsed >= RUN_MILLIS) { + clearInterval(timer); + console.log('[repro] completed without wedging (fix works)'); + } + }, 30000); +} + +let started = false; + +function tryStart() { + if (started) { + return; + } + try { + Java.perform(() => { + if (started) { + return; + } + started = true; + start(); + }); + } catch (e) { + console.log('[repro] Java.perform failed: ' + e); + } +} + +function start() { + startedAt = Date.now(); + installHooks(); + // allocator disabled for this A/B: JS-thread allocation wedges + // frida's attached thread under GC pressure regardless of the bridge + // (a repro artifact), so it is toggled via the ALLOC env flag below. + if (globalThis.ALLOC) { + startAllocator(); + } + startHeartbeat(); + console.log('[repro] running for ' + RUN_MILLIS + + ' ms; heartbeat stopping early = wedged (bug reproduced)'); +} + +tryStart(); +setInterval(tryStart, 250);