diff options
| author | Alice Ryhl <aliceryhl@google.com> | 2026-09-30 13:56:37 +0000 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2026-10-01 14:34:04 +0200 |
| commit | 831f1ddd64fa194eb7ddbb67ece363a57daa1537 (patch) | |
| tree | e0ca2f48a0911aa991256c59b554ff9fd9bddcf5 /drivers | |
| parent | 32d0ed324f6fc9428f2942c48ea5d2530e08665a (diff) | |
rust_binder: track caller location in BinderError
Record the caller's location in BinderError using #[track_caller] and a
new ErrorLocation wrapper around &'static Location<'static> across all
BinderError constructors and From implementations. ErrorLocation
implements Display by stripping the directory prefix so locations are
formatted as filename.rs:line (e.g. process.rs:422).
In Context::get_manager_node and Process::buffer_alloc, avoid passing
BinderError::new_dead() or BinderError::from() as function pointers to
Option::ok_or_else() and Result::map_err() so that #[track_caller]
captures the call site in rust_binder correctly.
Include the error location in the FailedTransaction debug print and
prepare for recording it in the binder transaction log. Example output:
rust_binder: 840:4583 transaction async to 7656:0 failed ESRCH, code 1 size 428-16 line thread.rs:711
Assisted-by: LLM
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Link: https://patch.msgid.link/20260930-binder-transaction-log-v2-2-8a15e9b8cae3@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers')
| -rw-r--r-- | drivers/android/binder/context.rs | 8 | ||||
| -rw-r--r-- | drivers/android/binder/error.rs | 39 | ||||
| -rw-r--r-- | drivers/android/binder/process.rs | 5 | ||||
| -rw-r--r-- | drivers/android/binder/thread.rs | 5 |
4 files changed, 50 insertions, 7 deletions
diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs index ddddb66b3557..bcc97ddaacd7 100644 --- a/drivers/android/binder/context.rs +++ b/drivers/android/binder/context.rs @@ -134,13 +134,13 @@ impl Context { } pub(crate) fn get_manager_node(&self, strong: bool) -> Result<NodeRef, BinderError> { - self.manager + Ok(self + .manager .lock() .node .as_ref() - .ok_or_else(BinderError::new_dead)? - .clone(strong) - .map_err(BinderError::from) + .ok_or_else(|| BinderError::new_dead())? + .clone(strong)?) } pub(crate) fn for_each_proc<F>(&self, mut func: F) diff --git a/drivers/android/binder/error.rs b/drivers/android/binder/error.rs index 1296072c35d9..41b07d687b30 100644 --- a/drivers/android/binder/error.rs +++ b/drivers/android/binder/error.rs @@ -2,6 +2,7 @@ // Copyright (C) 2025 Google LLC. +use core::panic::Location; use kernel::fmt; use kernel::prelude::*; @@ -9,32 +10,65 @@ use crate::defs::*; pub(crate) type BinderResult<T = ()> = core::result::Result<T, BinderError>; +/// Wraps a `&'static Location` to format it as `filename.rs:line` without the directory prefix. +#[derive(Copy, Clone)] +#[repr(transparent)] +pub(crate) struct ErrorLocation(&'static Location<'static>); + +// SAFETY: `ErrorLocation` is `repr(transparent)` over a shared reference, which is part of the +// option layout optimization guarantee, so all zeroes is a valid representation for `None`. +unsafe impl pin_init::ZeroableOption for ErrorLocation {} + +impl ErrorLocation { + #[track_caller] + pub(crate) const fn caller() -> Self { + Self(Location::caller()) + } +} + +impl fmt::Display for ErrorLocation { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let file = match self.0.file().rsplit_once('/') { + Some((_, file)) => file, + None => self.0.file(), + }; + write!(f, "{}:{}", file, self.0.line()) + } +} + /// An error that will be returned to userspace via the `BINDER_WRITE_READ` ioctl rather than via /// errno. pub(crate) struct BinderError { pub(crate) reply: u32, pub(crate) source: Option<Error>, + pub(crate) line: ErrorLocation, } impl BinderError { + #[track_caller] pub(crate) fn new_dead() -> Self { Self { reply: BR_DEAD_REPLY, source: None, + line: ErrorLocation::caller(), } } + #[track_caller] pub(crate) fn new_frozen() -> Self { Self { reply: BR_FROZEN_REPLY, source: None, + line: ErrorLocation::caller(), } } + #[track_caller] pub(crate) fn new_frozen_oneway() -> Self { Self { reply: BR_TRANSACTION_PENDING_FROZEN, source: None, + line: ErrorLocation::caller(), } } @@ -46,25 +80,30 @@ impl BinderError { /// Convert an errno into a `BinderError` and store the errno used to construct it. The errno /// should be stored as the thread's extended error when given to userspace. impl From<Error> for BinderError { + #[track_caller] fn from(source: Error) -> Self { Self { reply: BR_FAILED_REPLY, source: Some(source), + line: ErrorLocation::caller(), } } } impl From<kernel::fs::file::BadFdError> for BinderError { + #[track_caller] fn from(source: kernel::fs::file::BadFdError) -> Self { BinderError::from(Error::from(source)) } } impl From<kernel::alloc::AllocError> for BinderError { + #[track_caller] fn from(_: kernel::alloc::AllocError) -> Self { Self { reply: BR_FAILED_REPLY, source: Some(ENOMEM), + line: ErrorLocation::caller(), } } } diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index a56489d8bfcc..64960857bcdc 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -1071,7 +1071,10 @@ impl Process { let (new_alloc, addr) = loop { let mut inner = self.inner.lock(); - let mapping = inner.mapping.as_mut().ok_or_else(BinderError::new_dead)?; + let mapping = inner + .mapping + .as_mut() + .ok_or_else(|| BinderError::new_dead())?; let alloc_request = match mapping.alloc.reserve_new(reserve_new_args)? { ReserveNew::Success(new_alloc) => break (new_alloc, mapping.address), ReserveNew::NeedAlloc(request) => request, diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs index b5dece7d2285..78dc0ca6f630 100644 --- a/drivers/android/binder/thread.rs +++ b/drivers/android/binder/thread.rs @@ -1340,7 +1340,7 @@ impl Thread { binder_debug!( FailedTransaction, - "transaction {} to {}:{} failed {:?}, code {} size {}-{}", + "transaction {} to {}:{} failed {:?}, code {} size {}-{} line {}", if info.is_reply { "reply" } else if info.is_oneway() { @@ -1353,7 +1353,8 @@ impl Thread { err, info.code, info.data_size, - info.offsets_size + info.offsets_size, + err.line ); } } |
