github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-07-28 18:21:24 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

rust_binder: fix BINDER_GET_EXTENDED_ERROR

Browse Source 
This code currently copies the ExtendedError struct to the stack,
modifies the copy, and then doesn't modify the original. Thus, fix it.

Furthermore, errors when replying must be delivered directly to the
remote thread, so update deliver_reply() to take an extended error
argument.

Cc: stable <stable@kernel.org>
Fixes: eafedbc7c0 ("rust_binder: add Rust Binder driver")
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Acked-by: Carlos Llamas <cmllamas@google.com>
Link: https://patch.msgid.link/20260605-set-extended-error-v3-1-d60b69a75f97@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Alice Ryhl 2026-06-05 11:13:50 +00:00 committed by Greg Kroah-Hartman
parent dc59e4fea9
commit 77bfebf110
3 changed files with 58 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
drivers/android/binder/error.rs
View File

@ -73,20 +73,17 @@ impl fmt::Debug for BinderError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.reply { match self.reply {
BR_FAILED_REPLY => match self.source.as_ref() { BR_FAILED_REPLY => match self.source.as_ref() {
Some(source) => f Some(source) => source.fmt(f),
.debug_struct("BR_FAILED_REPLY")
.field("source", source)
.finish(),
None => f.pad("BR_FAILED_REPLY"), None => f.pad("BR_FAILED_REPLY"),
}, },
BR_DEAD_REPLY => f.pad("BR_DEAD_REPLY"), BR_DEAD_REPLY => f.pad("BR_DEAD_REPLY"),
BR_FROZEN_REPLY => f.pad("BR_FROZEN_REPLY"), BR_FROZEN_REPLY => f.pad("BR_FROZEN_REPLY"),
BR_TRANSACTION_PENDING_FROZEN => f.pad("BR_TRANSACTION_PENDING_FROZEN"), BR_TRANSACTION_PENDING_FROZEN => f.pad("BR_TRANSACTION_PENDING_FROZEN"),
BR_TRANSACTION_COMPLETE => f.pad("BR_TRANSACTION_COMPLETE"), BR_TRANSACTION_COMPLETE => f.pad("BR_TRANSACTION_COMPLETE"),
_ => f _ => match self.source.as_ref() {
.debug_struct("BinderError") Some(source) => source.fmt(f),
.field("reply", &self.reply) None => self.reply.fmt(f),
.finish(), },
} }
} }
} }

59
drivers/android/binder/thread.rs
View File

@ -495,9 +495,16 @@ pub(crate) fn debug_print(self: &Arc<Self>, m: &SeqFile, print_all: bool) -> Res
Ok(()) Ok(())
} }
pub(crate) fn clear_extended_error(&self, debug_id: usize) {
self.inner.lock().extended_error = ExtendedError::new(debug_id as u32, BR_OK, 0);
}
pub(crate) fn get_extended_error(&self, data: UserSlice) -> Result { pub(crate) fn get_extended_error(&self, data: UserSlice) -> Result {
let mut writer = data.writer(); let mut writer = data.writer();
let ee = self.inner.lock().extended_error; let mut inner = self.inner.lock();
let ee = inner.extended_error;
inner.extended_error = ExtendedError::new(0, BR_OK, 0);
drop(inner);
writer.write(&ee)?; writer.write(&ee)?;
Ok(()) Ok(())
} }
@ -1109,7 +1116,10 @@ fn unwind_transaction_stack(self: &Arc<Self>) {
inner.pop_transaction_to_reply(thread.as_ref()) inner.pop_transaction_to_reply(thread.as_ref())
} { } {
let reply = Err(BR_DEAD_REPLY); let reply = Err(BR_DEAD_REPLY);
if !transaction.from.deliver_single_reply(reply, &transaction) { if !transaction
.from
.deliver_single_reply(reply, &transaction, None)
{
break; break;
} }
@ -1121,8 +1131,9 @@ pub(crate) fn deliver_reply(
&self, &self,
reply: Result<DLArc<Transaction>, u32>, reply: Result<DLArc<Transaction>, u32>,
transaction: &DArc<Transaction>, transaction: &DArc<Transaction>,
extended_error: Option<ExtendedError>,
) { ) {
if self.deliver_single_reply(reply, transaction) { if self.deliver_single_reply(reply, transaction, extended_error) {
transaction.from.unwind_transaction_stack(); transaction.from.unwind_transaction_stack();
} }
} }
@ -1136,6 +1147,7 @@ fn deliver_single_reply(
&self, &self,
reply: Result<DLArc<Transaction>, u32>, reply: Result<DLArc<Transaction>, u32>,
transaction: &DArc<Transaction>, transaction: &DArc<Transaction>,
extended_error: Option<ExtendedError>,
) -> bool { ) -> bool {
if let Ok(transaction) = &reply { if let Ok(transaction) = &reply {
crate::trace::trace_transaction(true, transaction, Some(&self.task)); crate::trace::trace_transaction(true, transaction, Some(&self.task));
@ -1152,6 +1164,12 @@ fn deliver_single_reply(
return true; return true;
} }
if let Some(ee) = extended_error {
if inner.extended_error.command == BR_OK {
inner.extended_error = ee;
}
}
match reply { match reply {
Ok(work) => { Ok(work) => {
inner.push_work(work); inner.push_work(work);
@ -1222,6 +1240,9 @@ fn read_transaction_info(
info.buffers_size = td.buffers_size as usize; info.buffers_size = td.buffers_size as usize;
// SAFETY: Above `read` call initializes all bytes, so this union read is ok. // SAFETY: Above `read` call initializes all bytes, so this union read is ok.
info.target_handle = unsafe { td.transaction_data.target.handle }; info.target_handle = unsafe { td.transaction_data.target.handle };
info.debug_id = super::next_debug_id();
Ok(()) Ok(())
} }
@ -1230,6 +1251,8 @@ fn transaction(self: &Arc<Self>, cmd: u32, reader: &mut UserSliceReader) -> Resu
let mut info = TransactionInfo::zeroed(); let mut info = TransactionInfo::zeroed();
self.read_transaction_info(cmd, reader, &mut info)?; self.read_transaction_info(cmd, reader, &mut info)?;
self.clear_extended_error(info.debug_id);
let ret = if info.is_reply { let ret = if info.is_reply {
self.reply_inner(&mut info) self.reply_inner(&mut info)
} else if info.is_oneway() { } else if info.is_oneway() {
@ -1239,23 +1262,21 @@ fn transaction(self: &Arc<Self>, cmd: u32, reader: &mut UserSliceReader) -> Resu
}; };
if let Err(err) = ret { if let Err(err) = ret {
self.push_return_work(err.reply);
if err.reply != BR_TRANSACTION_COMPLETE { if err.reply != BR_TRANSACTION_COMPLETE {
info.reply = err.reply; info.reply = err.reply;
}
self.push_return_work(err.reply);
if let Some(source) = &err.source { if let Some(source) = &err.source {
info.errno = source.to_errno(); info.errno = source.to_errno();
info.reply = err.reply;
{ {
let mut ee = self.inner.lock().extended_error; let mut inner = self.inner.lock();
ee.command = err.reply; inner.extended_error =
ee.param = source.to_errno(); ExtendedError::new(info.debug_id as u32, err.reply, source.to_errno());
}
} }
pr_warn!( pr_warn!(
"{}:{} transaction to {} failed: {source:?}", "{}:{} transaction to {} failed: {err:?}",
info.from_pid, info.from_pid,
info.from_tid, info.from_tid,
info.to_pid info.to_pid
@ -1320,18 +1341,24 @@ fn reply_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResult {
let allow_fds = orig.flags & TF_ACCEPT_FDS != 0; let allow_fds = orig.flags & TF_ACCEPT_FDS != 0;
let reply = Transaction::new_reply(self, process, info, allow_fds)?; let reply = Transaction::new_reply(self, process, info, allow_fds)?;
self.inner.lock().push_work(completion); self.inner.lock().push_work(completion);
orig.from.deliver_reply(Ok(reply), &orig); orig.from.deliver_reply(Ok(reply), &orig, None);
Ok(()) Ok(())
})() })()
.map_err(|mut err| { .map_err(|mut err| {
// At this point we only return `BR_TRANSACTION_COMPLETE` to the caller, and we must let // At this point we only return `BR_TRANSACTION_COMPLETE` to the caller, and we must let
// the sender know that the transaction has completed (with an error in this case). // the sender know that the transaction has completed (with an error in this case).
pr_warn!( pr_warn!(
"Failure {:?} during reply - delivering BR_FAILED_REPLY to sender.", "{}:{} reply to {} failed: {err:?}",
err info.from_pid,
info.from_tid,
info.to_pid
); );
let reply = Err(BR_FAILED_REPLY);
orig.from.deliver_reply(reply, &orig); let param = err.source.as_ref().map_or(0, |e| e.to_errno());
let ee = ExtendedError::new(info.debug_id as u32, err.reply, param);
orig.from
.deliver_reply(Err(BR_FAILED_REPLY), &orig, Some(ee));
err.reply = BR_TRANSACTION_COMPLETE; err.reply = BR_TRANSACTION_COMPLETE;
err err
}); });

15
drivers/android/binder/transaction.rs
View File

@ -42,6 +42,7 @@ pub(crate) struct TransactionInfo {
pub(crate) reply: u32, pub(crate) reply: u32,
pub(crate) oneway_spam_suspect: bool, pub(crate) oneway_spam_suspect: bool,
pub(crate) is_reply: bool, pub(crate) is_reply: bool,
pub(crate) debug_id: usize,
} }
impl TransactionInfo { impl TransactionInfo {
@ -93,7 +94,6 @@ pub(crate) fn new(
from: &Arc<Thread>, from: &Arc<Thread>,
info: &mut TransactionInfo, info: &mut TransactionInfo,
) -> BinderResult<DLArc<Self>> { ) -> BinderResult<DLArc<Self>> {
let debug_id = super::next_debug_id();
let allow_fds = node_ref.node.flags & FLAT_BINDER_FLAG_ACCEPTS_FDS != 0; let allow_fds = node_ref.node.flags & FLAT_BINDER_FLAG_ACCEPTS_FDS != 0;
let txn_security_ctx = node_ref.node.flags & FLAT_BINDER_FLAG_TXN_SECURITY_CTX != 0; let txn_security_ctx = node_ref.node.flags & FLAT_BINDER_FLAG_TXN_SECURITY_CTX != 0;
let mut txn_security_ctx_off = if txn_security_ctx { Some(0) } else { None }; let mut txn_security_ctx_off = if txn_security_ctx { Some(0) } else { None };
@ -101,7 +101,7 @@ pub(crate) fn new(
let mut alloc = match from.copy_transaction_data( let mut alloc = match from.copy_transaction_data(
to.clone(), to.clone(),
info, info,
debug_id, info.debug_id,
allow_fds, allow_fds,
txn_security_ctx_off.as_mut(), txn_security_ctx_off.as_mut(),
) { ) {
@ -128,7 +128,7 @@ pub(crate) fn new(
let data_address = alloc.ptr; let data_address = alloc.ptr;
Ok(DTRWrap::arc_pin_init(pin_init!(Transaction { Ok(DTRWrap::arc_pin_init(pin_init!(Transaction {
debug_id, debug_id: info.debug_id,
target_node: Some(target_node), target_node: Some(target_node),
from_parent, from_parent,
sender_euid: Kuid::current_euid(), sender_euid: Kuid::current_euid(),
@ -152,9 +152,8 @@ pub(crate) fn new_reply(
info: &mut TransactionInfo, info: &mut TransactionInfo,
allow_fds: bool, allow_fds: bool,
) -> BinderResult<DLArc<Self>> { ) -> BinderResult<DLArc<Self>> {
let debug_id = super::next_debug_id();
let mut alloc = let mut alloc =
match from.copy_transaction_data(to.clone(), info, debug_id, allow_fds, None) { match from.copy_transaction_data(to.clone(), info, info.debug_id, allow_fds, None) {
Ok(alloc) => alloc, Ok(alloc) => alloc,
Err(err) => { Err(err) => {
pr_warn!("Failure in copy_transaction_data: {:?}", err); pr_warn!("Failure in copy_transaction_data: {:?}", err);
@ -165,7 +164,7 @@ pub(crate) fn new_reply(
alloc.set_info_clear_on_drop(); alloc.set_info_clear_on_drop();
} }
Ok(DTRWrap::arc_pin_init(pin_init!(Transaction { Ok(DTRWrap::arc_pin_init(pin_init!(Transaction {
debug_id, debug_id: info.debug_id,
target_node: None, target_node: None,
from_parent: None, from_parent: None,
sender_euid: Kuid::current_euid(), sender_euid: Kuid::current_euid(),
@ -394,7 +393,7 @@ fn do_work(
let send_failed_reply = ScopeGuard::new(|| { let send_failed_reply = ScopeGuard::new(|| {
if self.target_node.is_some() && self.flags & TF_ONE_WAY == 0 { if self.target_node.is_some() && self.flags & TF_ONE_WAY == 0 {
let reply = Err(BR_FAILED_REPLY); let reply = Err(BR_FAILED_REPLY);
self.from.deliver_reply(reply, &self); self.from.deliver_reply(reply, &self, None);
} }
self.drop_outstanding_txn(); self.drop_outstanding_txn();
}); });
@ -478,7 +477,7 @@ fn cancel(self: DArc<Self>) {
// If this is not a reply or oneway transaction, then send a dead reply. // If this is not a reply or oneway transaction, then send a dead reply.
if self.target_node.is_some() && self.flags & TF_ONE_WAY == 0 { if self.target_node.is_some() && self.flags & TF_ONE_WAY == 0 {
let reply = Err(BR_DEAD_REPLY); let reply = Err(BR_DEAD_REPLY);
self.from.deliver_reply(reply, &self); self.from.deliver_reply(reply, &self, None);
} }
self.drop_outstanding_txn(); self.drop_outstanding_txn();