Skip to content

Commit de7ee67

Browse files
committed
rust_binder: Implement BINDER_DEBUG_DEAD_TRANSACTION
This adds dynamic debug logs for: - Releasing active transactions during thread stack unwinding. - Discarded transaction error codes when a thread exits. - Undelivered transaction acknowledgments (TRANSACTION_COMPLETE) upon thread exit. - Undelivered process death and freeze notifications when processes exit or die. - Undelivered transactions canceled due to target process death. We now store the process PID in `ThreadError`, `DeliverCode`, and `FreezeMessage` to ensure the correct PID is logged on cancellation. This is necessary because `cancel()` runs from background `kworkers`, which would otherwise print the wrong PID. Signed-off-by: Jahnavi MN <jahnavimn@google.com>
1 parent 83ca110 commit de7ee67

5 files changed

Lines changed: 76 additions & 20 deletions

File tree

drivers/android/binder/freeze.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ type UninitFM = UniqueArc<core::mem::MaybeUninit<DTRWrap<FreezeMessage>>>;
6060
/// Represents a notification that the freeze state has changed.
6161
pub(crate) struct FreezeMessage {
6262
cookie: FreezeCookie,
63+
pid: i32,
6364
}
6465

6566
kernel::list::impl_list_arc_safe! {
@@ -73,8 +74,8 @@ impl FreezeMessage {
7374
UniqueArc::new_uninit(flags)
7475
}
7576

76-
fn init(ua: UninitFM, cookie: FreezeCookie) -> DLArc<FreezeMessage> {
77-
match ua.pin_init_with(DTRWrap::new(FreezeMessage { cookie })) {
77+
fn init(ua: UninitFM, cookie: FreezeCookie, pid: i32) -> DLArc<FreezeMessage> {
78+
match ua.pin_init_with(DTRWrap::new(FreezeMessage { cookie, pid })) {
7879
Ok(msg) => ListArc::from(msg),
7980
Err(err) => match err {},
8081
}
@@ -140,7 +141,14 @@ impl DeliverToRead for FreezeMessage {
140141
}
141142
}
142143

143-
fn cancel(self: DArc<Self>) {}
144+
fn cancel(self: DArc<Self>) {
145+
binder_debug!(
146+
pid = self.pid,
147+
DeadTransaction,
148+
"undelivered freeze notification, {:016x}",
149+
self.cookie.0
150+
);
151+
}
144152

145153
fn should_sync_wakeup(&self) -> bool {
146154
false
@@ -251,7 +259,7 @@ impl Process {
251259
}
252260

253261
*info.freeze() = Some(cookie);
254-
let msg = FreezeMessage::init(msg, cookie);
262+
let msg = FreezeMessage::init(msg, cookie, self.task.pid());
255263
drop(node_refs_guard);
256264
let _ = self.push_work(msg);
257265
Ok(())
@@ -272,7 +280,7 @@ impl Process {
272280
};
273281
let mut clear_msg = None;
274282
if freeze.num_pending_duplicates > 0 {
275-
clear_msg = Some(FreezeMessage::init(alloc, cookie));
283+
clear_msg = Some(FreezeMessage::init(alloc, cookie, self.task.pid()));
276284
freeze.num_pending_duplicates -= 1;
277285
freeze.num_cleared_duplicates += 1;
278286
} else {
@@ -287,7 +295,7 @@ impl Process {
287295
let is_frozen = freeze.node.owner.inner.lock().is_frozen.is_fully_frozen();
288296
if freeze.is_clearing || freeze.last_is_frozen != Some(is_frozen) {
289297
// Immediately send another FreezeMessage.
290-
clear_msg = Some(FreezeMessage::init(alloc, cookie));
298+
clear_msg = Some(FreezeMessage::init(alloc, cookie, self.task.pid()));
291299
}
292300
freeze.is_pending = false;
293301
}
@@ -340,7 +348,7 @@ impl Process {
340348
*info.freeze() = None;
341349
let mut msg = None;
342350
if !listener.is_pending {
343-
msg = Some(FreezeMessage::init(alloc, cookie));
351+
msg = Some(FreezeMessage::init(alloc, cookie, self.task.pid()));
344352
}
345353
drop(node_refs_guard);
346354

@@ -420,7 +428,7 @@ impl Process {
420428
continue;
421429
};
422430
let msg_alloc = FreezeMessage::new(GFP_KERNEL)?;
423-
let msg = FreezeMessage::init(msg_alloc, cookie);
431+
let msg = FreezeMessage::init(msg_alloc, cookie, proc.task.pid());
424432
batch.push((proc, msg), GFP_KERNEL)?;
425433
}
426434

drivers/android/binder/node.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1120,7 +1120,14 @@ impl DeliverToRead for NodeDeath {
11201120
Ok(cmd != BR_DEAD_BINDER)
11211121
}
11221122

1123-
fn cancel(self: DArc<Self>) {}
1123+
fn cancel(self: DArc<Self>) {
1124+
binder_debug!(
1125+
pid = self.process.task.pid(),
1126+
DeadTransaction,
1127+
"undelivered death notification, {:016x}",
1128+
self.cookie
1129+
);
1130+
}
11241131

11251132
fn should_sync_wakeup(&self) -> bool {
11261133
false

drivers/android/binder/rust_binder_main.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,17 +221,19 @@ impl<T: ListArcSafe> DTRWrap<T> {
221221
struct DeliverCode {
222222
code: u32,
223223
skip: Atomic<bool>,
224+
pid: i32,
224225
}
225226

226227
kernel::list::impl_list_arc_safe! {
227228
impl ListArcSafe<0> for DeliverCode { untracked; }
228229
}
229230

230231
impl DeliverCode {
231-
fn new(code: u32) -> Self {
232+
fn new(code: u32, pid: i32) -> Self {
232233
Self {
233234
code,
234235
skip: Atomic::new(false),
236+
pid,
235237
}
236238
}
237239

@@ -256,7 +258,15 @@ impl DeliverToRead for DeliverCode {
256258
Ok(true)
257259
}
258260

259-
fn cancel(self: DArc<Self>) {}
261+
fn cancel(self: DArc<Self>) {
262+
if !self.skip.load(Relaxed) {
263+
binder_debug!(
264+
pid = self.pid,
265+
DeadTransaction,
266+
"undelivered TRANSACTION_COMPLETE"
267+
);
268+
}
269+
}
260270

261271
fn should_sync_wakeup(&self) -> bool {
262272
false

drivers/android/binder/thread.rs

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ const LOOPER_WAITING_PROC: u32 = 0x20;
279279
const LOOPER_POLL: u32 = 0x40;
280280

281281
impl InnerThread {
282-
fn new() -> Result<Self> {
282+
fn new(pid: i32) -> Result<Self> {
283283
fn next_err_id() -> u32 {
284284
static EE_ID: Atomic<u32> = Atomic::new(0);
285285
EE_ID.fetch_add(1, Relaxed)
@@ -290,8 +290,8 @@ impl InnerThread {
290290
looper_need_return: false,
291291
is_dead: false,
292292
process_work_list: false,
293-
reply_work: ThreadError::try_new()?,
294-
return_work: ThreadError::try_new()?,
293+
reply_work: ThreadError::try_new(pid)?,
294+
return_work: ThreadError::try_new(pid)?,
295295
work_list: List::new(),
296296
current_transaction: None,
297297
extended_error: ExtendedError::new(next_err_id(), BR_OK, 0),
@@ -445,7 +445,7 @@ kernel::list::impl_list_item! {
445445

446446
impl Thread {
447447
pub(crate) fn new(id: i32, process: Arc<Process>) -> Result<Arc<Self>> {
448-
let inner = InnerThread::new()?;
448+
let inner = InnerThread::new(process.task.pid())?;
449449

450450
Arc::pin_init(
451451
try_pin_init!(Thread {
@@ -1108,6 +1108,12 @@ impl Thread {
11081108
let mut inner = thread.inner.lock();
11091109
inner.pop_transaction_to_reply(thread.as_ref())
11101110
} {
1111+
binder_debug!(
1112+
DeadTransaction,
1113+
"release transaction {} in, still active",
1114+
transaction.debug_id
1115+
);
1116+
11111117
let reply = Err(BR_DEAD_REPLY);
11121118
if !transaction.from.deliver_single_reply(reply, &transaction) {
11131119
break;
@@ -1284,7 +1290,10 @@ impl Thread {
12841290
// TODO: We need to ensure that there isn't a pending transaction in the work queue. How
12851291
// could this happen?
12861292
let top = self.top_of_transaction_stack()?;
1287-
let list_completion = DTRWrap::arc_try_new(DeliverCode::new(BR_TRANSACTION_COMPLETE))?;
1293+
let list_completion = DTRWrap::arc_try_new(DeliverCode::new(
1294+
BR_TRANSACTION_COMPLETE,
1295+
self.process.task.pid(),
1296+
))?;
12881297
let completion = list_completion.clone_arc();
12891298
let transaction = Transaction::new(node_ref, top, self, info)?;
12901299

@@ -1336,7 +1345,10 @@ impl Thread {
13361345

13371346
// We need to complete the transaction even if we cannot complete building the reply.
13381347
let out = (|| -> BinderResult<_> {
1339-
let completion = DTRWrap::arc_try_new(DeliverCode::new(BR_TRANSACTION_COMPLETE))?;
1348+
let completion = DTRWrap::arc_try_new(DeliverCode::new(
1349+
BR_TRANSACTION_COMPLETE,
1350+
self.process.task.pid(),
1351+
))?;
13401352
let process = orig.from.process.clone();
13411353
let allow_fds = orig.flags & TF_ACCEPT_FDS != 0;
13421354
let reply = Transaction::new_reply(self, process, info, allow_fds)?;
@@ -1370,7 +1382,8 @@ impl Thread {
13701382
} else {
13711383
BR_TRANSACTION_COMPLETE
13721384
};
1373-
let list_completion = DTRWrap::arc_try_new(DeliverCode::new(code))?;
1385+
let list_completion =
1386+
DTRWrap::arc_try_new(DeliverCode::new(code, self.process.task.pid()))?;
13741387
let completion = list_completion.clone_arc();
13751388
self.inner.lock().push_work(list_completion);
13761389
match transaction.submit(info) {
@@ -1626,14 +1639,16 @@ impl Thread {
16261639
#[pin_data]
16271640
struct ThreadError {
16281641
error_code: Atomic<u32>,
1642+
pid: i32,
16291643
#[pin]
16301644
links_track: AtomicTracker,
16311645
}
16321646

16331647
impl ThreadError {
1634-
fn try_new() -> Result<DArc<Self>> {
1648+
fn try_new(pid: i32) -> Result<DArc<Self>> {
16351649
DTRWrap::arc_pin_init(pin_init!(Self {
16361650
error_code: Atomic::new(BR_OK),
1651+
pid,
16371652
links_track <- AtomicTracker::new(),
16381653
}))
16391654
.map(ListArc::into_arc)
@@ -1660,7 +1675,16 @@ impl DeliverToRead for ThreadError {
16601675
Ok(true)
16611676
}
16621677

1663-
fn cancel(self: DArc<Self>) {}
1678+
fn cancel(self: DArc<Self>) {
1679+
let code = self.error_code.load(Relaxed);
1680+
if code != BR_OK {
1681+
binder_debug!(
1682+
pid = self.pid,
1683+
DeadTransaction,
1684+
"undelivered TRANSACTION_ERROR: {code}"
1685+
);
1686+
}
1687+
}
16641688

16651689
fn should_sync_wakeup(&self) -> bool {
16661690
false

drivers/android/binder/transaction.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,13 @@ impl DeliverToRead for Transaction {
489489
if self.target_node.is_some() && self.flags & TF_ONE_WAY == 0 {
490490
let reply = Err(BR_DEAD_REPLY);
491491
self.from.deliver_reply(reply, &self);
492+
} else {
493+
binder_debug!(
494+
pid = self.to.task.pid(),
495+
DeadTransaction,
496+
"undelivered transaction {}, process died",
497+
self.debug_id
498+
);
492499
}
493500

494501
self.drop_outstanding_txn();

0 commit comments

Comments
 (0)