Describe the bug
On Android, droidplug's FnAdapter can be woken and closed concurrently from two threads. jni-rs's take_rust_field turns that race into a use-after-free, which crashes the process with SIGSEGV or SIGBUS in btleplug::droidplug::jni_utils::ops::fn_adapter_call_internal.
This is the top native crash in Intiface Central on Android. It shows up at a roughly 1.1% crash rate, flat across btleplug 0.12.0 (jni 0.19) and 0.13.2 (jni 0.22.4), so the jni 0.22 migration did not change it.
Expected behavior
FnAdapter.call and FnAdapter.close can run concurrently on the same adapter without undefined behaviour. A call that races a close either runs the closure or returns null.
Actual behavior
The process crashes in native code. Representative stack from btleplug 0.13.2:
#00 librust_lib_intiface_central.so jni::env::EnvUnowned::with_env
#01 librust_lib_intiface_central.so btleplug::droidplug::jni_utils::ops::fn_adapter_call_internal
#03 io.github.gedgygedgy.rust.ops.FnAdapter.call
#05 io.github.gedgygedgy.rust.ops.FnRunnableImpl.run
io.github.gedgygedgy.rust.task.Waker.wake (in some reports)
#06 io.github.gedgygedgy.rust.stream.QueueStream.doEvent
#07 com.nonpolynomial.btleplug.android.impl.Peripheral$Callback.onCharacteristicChanged
#09 android.bluetooth.BluetoothGattCallback.onCharacteristicChanged
... binder thread
A smaller group of reports crashes in btleplug::droidplug::jni_utils::exceptions::throw_unwind, which fits a panic from lock().unwrap() on freed memory. These reports come from a wide range of devices (Pixel, Samsung S22/S23/S25/A54, Oppo) on API 34 to 37.
Additional context
Mechanism:
- Java race (
src/droidplug/java/.../rust/stream/QueueStream.java):
doEvent runs on the Android binder thread. It reads this.waker under this.lock, then calls waker.wake() after releasing the lock.
pollNext runs on the Rust poll thread. It swaps in a new waker under the lock, then calls oldWaker.close() outside the lock.
- So
FnAdapter.call and FnAdapter.close can hit the same adapter at the same time. SimpleFuture has the same pattern.
- Rust side (
src/droidplug/jni_utils/ops.rs): fn_adapter_call_internal uses get_rust_field and clones the Arc while holding the returned MutexGuard. fn_adapter_close_internal uses take_rust_field.
- jni-rs 0.22.4 (
src/env.rs):
get_rust_field releases the Java monitor on return but hands back a MutexGuard into the heap-allocated Mutex<T>.
take_rust_field does Box::from_raw(ptr) and then drop(mbox.try_lock()?). If a guard is outstanding, try_lock fails and the ? early return drops the Box. That frees the Mutex while the other thread still holds its guard.
- The Java field is not zeroed on that path either, so later calls dereference freed memory.
Proposed fix: stop using get_rust_field / take_rust_field for FnAdapter and store Arc::into_raw in the data field instead.
- call: lock the object monitor, read the pointer, and if it is non-null do
Arc::increment_strong_count + Arc::from_raw. Then unlock and invoke. An in-flight call owns its own reference.
- close: lock the monitor, read and zero the field, unlock, then drop the
Arc.
This is sound regardless of Java-side concurrency. It should come with a stress test that runs wake and close concurrently on the same adapter. We should also audit the other get_rust_field / take_rust_field users in droidplug for the same pattern.
The take_rust_field failure path is a separate soundness bug in jni-rs and should be reported upstream as well.
Describe the bug
On Android,
droidplug'sFnAdaptercan be woken and closed concurrently from two threads. jni-rs'stake_rust_fieldturns that race into a use-after-free, which crashes the process with SIGSEGV or SIGBUS inbtleplug::droidplug::jni_utils::ops::fn_adapter_call_internal.This is the top native crash in Intiface Central on Android. It shows up at a roughly 1.1% crash rate, flat across btleplug 0.12.0 (jni 0.19) and 0.13.2 (jni 0.22.4), so the jni 0.22 migration did not change it.
Expected behavior
FnAdapter.callandFnAdapter.closecan run concurrently on the same adapter without undefined behaviour. A call that races a close either runs the closure or returns null.Actual behavior
The process crashes in native code. Representative stack from btleplug 0.13.2:
A smaller group of reports crashes in
btleplug::droidplug::jni_utils::exceptions::throw_unwind, which fits a panic fromlock().unwrap()on freed memory. These reports come from a wide range of devices (Pixel, Samsung S22/S23/S25/A54, Oppo) on API 34 to 37.Additional context
Mechanism:
src/droidplug/java/.../rust/stream/QueueStream.java):doEventruns on the Android binder thread. It readsthis.wakerunderthis.lock, then callswaker.wake()after releasing the lock.pollNextruns on the Rust poll thread. It swaps in a new waker under the lock, then callsoldWaker.close()outside the lock.FnAdapter.callandFnAdapter.closecan hit the same adapter at the same time.SimpleFuturehas the same pattern.src/droidplug/jni_utils/ops.rs):fn_adapter_call_internalusesget_rust_fieldand clones theArcwhile holding the returnedMutexGuard.fn_adapter_close_internalusestake_rust_field.src/env.rs):get_rust_fieldreleases the Java monitor on return but hands back aMutexGuardinto the heap-allocatedMutex<T>.take_rust_fielddoesBox::from_raw(ptr)and thendrop(mbox.try_lock()?). If a guard is outstanding,try_lockfails and the?early return drops the Box. That frees theMutexwhile the other thread still holds its guard.Proposed fix: stop using
get_rust_field/take_rust_fieldforFnAdapterand storeArc::into_rawin thedatafield instead.Arc::increment_strong_count+Arc::from_raw. Then unlock and invoke. An in-flight call owns its own reference.Arc.This is sound regardless of Java-side concurrency. It should come with a stress test that runs
wakeandcloseconcurrently on the same adapter. We should also audit the otherget_rust_field/take_rust_fieldusers in droidplug for the same pattern.The
take_rust_fieldfailure path is a separate soundness bug in jni-rs and should be reported upstream as well.