diff options
Diffstat (limited to 'drivers/android/binder')
| -rw-r--r-- | drivers/android/binder/error.rs | 5 | ||||
| -rw-r--r-- | drivers/android/binder/freeze.rs | 22 | ||||
| -rw-r--r-- | drivers/android/binder/node.rs | 2 | ||||
| -rw-r--r-- | drivers/android/binder/process.rs | 118 | ||||
| -rw-r--r-- | drivers/android/binder/range_alloc/tree.rs | 2 | ||||
| -rw-r--r-- | drivers/android/binder/rust_binderfs.c | 121 | ||||
| -rw-r--r-- | drivers/android/binder/stats.rs | 6 | ||||
| -rw-r--r-- | drivers/android/binder/transaction.rs | 6 |
8 files changed, 146 insertions, 136 deletions
diff --git a/drivers/android/binder/error.rs b/drivers/android/binder/error.rs index 9921827267d0..b24497cfa292 100644 --- a/drivers/android/binder/error.rs +++ b/drivers/android/binder/error.rs @@ -2,6 +2,7 @@ // Copyright (C) 2025 Google LLC. +use kernel::fmt; use kernel::prelude::*; use crate::defs::*; @@ -76,8 +77,8 @@ impl From<kernel::alloc::AllocError> for BinderError { } } -impl core::fmt::Debug for BinderError { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { +impl fmt::Debug for BinderError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.reply { BR_FAILED_REPLY => match self.source.as_ref() { Some(source) => f diff --git a/drivers/android/binder/freeze.rs b/drivers/android/binder/freeze.rs index e68c3c8bc55a..53b60035639a 100644 --- a/drivers/android/binder/freeze.rs +++ b/drivers/android/binder/freeze.rs @@ -106,13 +106,22 @@ impl DeliverToRead for FreezeMessage { return Ok(true); } if freeze.is_clearing { - _removed_listener = freeze_entry.remove_node(); + kernel::warn_on!(freeze.num_cleared_duplicates != 0); + if freeze.num_pending_duplicates > 0 { + // The primary freeze listener was deleted, so convert a pending duplicate back + // into the primary one. + freeze.num_pending_duplicates -= 1; + freeze.is_pending = true; + freeze.is_clearing = true; + } else { + _removed_listener = freeze_entry.remove_node(); + } drop(node_refs); writer.write_code(BR_CLEAR_FREEZE_NOTIFICATION_DONE)?; writer.write_payload(&self.cookie.0)?; Ok(true) } else { - let is_frozen = freeze.node.owner.inner.lock().is_frozen; + let is_frozen = freeze.node.owner.inner.lock().is_frozen.is_fully_frozen(); if freeze.last_is_frozen == Some(is_frozen) { return Ok(true); } @@ -245,8 +254,9 @@ impl Process { ); return Err(EINVAL); } - if freeze.is_clearing { - // Immediately send another FreezeMessage for BR_CLEAR_FREEZE_NOTIFICATION_DONE. + let is_frozen = freeze.node.owner.inner.lock().is_frozen.is_fully_frozen(); + if freeze.is_clearing || freeze.last_is_frozen != Some(is_frozen) { + // Immediately send another FreezeMessage. clear_msg = Some(FreezeMessage::init(alloc, cookie)); } freeze.is_pending = false; @@ -321,7 +331,7 @@ impl Process { KVVec::with_capacity(8, GFP_KERNEL).unwrap_or_else(|_err| KVVec::new()); let mut inner = self.lock_with_nodes(); - let mut curr = inner.nodes.cursor_front(); + let mut curr = inner.nodes.cursor_front_mut(); while let Some(cursor) = curr { let (key, node) = cursor.current(); let key = *key; @@ -335,7 +345,7 @@ impl Process { // Find the node we were looking at and try again. If the set of nodes was changed, // then just proceed to the next node. This is ok because we don't guarantee the // inclusion of nodes that are added or removed in parallel with this operation. - curr = inner.nodes.cursor_lower_bound(&key); + curr = inner.nodes.cursor_lower_bound_mut(&key); continue; } diff --git a/drivers/android/binder/node.rs b/drivers/android/binder/node.rs index ade895ef791e..08d362deaf61 100644 --- a/drivers/android/binder/node.rs +++ b/drivers/android/binder/node.rs @@ -687,7 +687,7 @@ impl Node { ); } if inner.freeze_list.is_empty() { - _unused_capacity = mem::replace(&mut inner.freeze_list, KVVec::new()); + _unused_capacity = mem::take(&mut inner.freeze_list); } } diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index f13a747e784c..ac981614544e 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -19,6 +19,7 @@ use kernel::{ cred::Credential, error::Error, fs::file::{self, File}, + id_pool::IdPool, list::{List, ListArc, ListArcField, ListLinks}, mm, prelude::*, @@ -72,6 +73,33 @@ impl Mapping { const PROC_DEFER_FLUSH: u8 = 1; const PROC_DEFER_RELEASE: u8 = 2; +#[derive(Copy, Clone)] +pub(crate) enum IsFrozen { + Yes, + No, + InProgress, +} + +impl IsFrozen { + /// Whether incoming transactions should be rejected due to freeze. + pub(crate) fn is_frozen(self) -> bool { + match self { + IsFrozen::Yes => true, + IsFrozen::No => false, + IsFrozen::InProgress => true, + } + } + + /// Whether freeze notifications consider this process frozen. + pub(crate) fn is_fully_frozen(self) -> bool { + match self { + IsFrozen::Yes => true, + IsFrozen::No => false, + IsFrozen::InProgress => false, + } + } +} + /// The fields of `Process` protected by the spinlock. pub(crate) struct ProcessInner { is_manager: bool, @@ -98,7 +126,7 @@ pub(crate) struct ProcessInner { /// are woken up. outstanding_txns: u32, /// Process is frozen and unable to service binder transactions. - pub(crate) is_frozen: bool, + pub(crate) is_frozen: IsFrozen, /// Process received sync transactions since last frozen. pub(crate) sync_recv: bool, /// Process received async transactions since last frozen. @@ -124,7 +152,7 @@ impl ProcessInner { started_thread_count: 0, defer_work: 0, outstanding_txns: 0, - is_frozen: false, + is_frozen: IsFrozen::No, sync_recv: false, async_recv: false, binderfs_file: None, @@ -367,6 +395,8 @@ kernel::list::impl_list_item! { struct ProcessNodeRefs { /// Used to look up nodes using the 32-bit id that this process knows it by. by_handle: RBTree<u32, ListArc<NodeRefInfo, { NodeRefInfo::LIST_PROC }>>, + /// Used to quickly find unused ids in `by_handle`. + handle_is_present: IdPool, /// Used to look up nodes without knowing their local 32-bit id. The usize is the address of /// the underlying `Node` struct as returned by `Node::global_id`. by_node: RBTree<usize, u32>, @@ -381,6 +411,7 @@ impl ProcessNodeRefs { fn new() -> Self { Self { by_handle: RBTree::new(), + handle_is_present: IdPool::new(), by_node: RBTree::new(), freeze_listeners: RBTree::new(), } @@ -596,7 +627,7 @@ impl Process { " ref {}: desc {} {}node {debug_id} s {strong} w {weak}", r.debug_id, r.handle, - if dead { "dead " } else { "" }, + if dead { "dead " } else { "" } ); } } @@ -775,7 +806,7 @@ impl Process { pub(crate) fn insert_or_update_handle( self: ArcBorrow<'_, Process>, node_ref: NodeRef, - is_mananger: bool, + is_manager: bool, ) -> Result<u32> { { let mut refs = self.node_refs.lock(); @@ -794,7 +825,33 @@ impl Process { let reserve2 = RBTreeNodeReservation::new(GFP_KERNEL)?; let info = UniqueArc::new_uninit(GFP_KERNEL)?; - let mut refs = self.node_refs.lock(); + let mut refs_lock = self.node_refs.lock(); + let mut refs = &mut *refs_lock; + + let (unused_id, by_handle_slot) = loop { + // ID 0 may only be used by the manager. + let start = if is_manager { 0 } else { 1 }; + + if let Some(res) = refs.handle_is_present.find_unused_id(start) { + match refs.by_handle.entry(res.as_u32()) { + rbtree::Entry::Vacant(entry) => break (res, entry), + rbtree::Entry::Occupied(_) => { + pr_err!("Detected mismatch between handle_is_present and by_handle"); + res.acquire(); + kernel::warn_on!(true); + return Err(EINVAL); + } + } + } + + let grow_request = refs.handle_is_present.grow_request().ok_or(ENOMEM)?; + drop(refs_lock); + let resizer = grow_request.realloc(GFP_KERNEL)?; + refs_lock = self.node_refs.lock(); + refs = &mut *refs_lock; + refs.handle_is_present.grow(resizer); + }; + let handle = unused_id.as_u32(); // Do a lookup again as node may have been inserted before the lock was reacquired. if let Some(handle_ref) = refs.by_node.get(&node_ref.node.global_id()) { @@ -804,20 +861,9 @@ impl Process { return Ok(handle); } - // Find id. - let mut target: u32 = if is_mananger { 0 } else { 1 }; - for handle in refs.by_handle.keys() { - if *handle > target { - break; - } - if *handle == target { - target = target.checked_add(1).ok_or(ENOMEM)?; - } - } - let gid = node_ref.node.global_id(); let (info_proc, info_node) = { - let info_init = NodeRefInfo::new(node_ref, target, self.into()); + let info_init = NodeRefInfo::new(node_ref, handle, self.into()); match info.pin_init_with(info_init) { Ok(info) => ListArc::pair_from_pin_unique(info), // error is infallible @@ -838,9 +884,10 @@ impl Process { // `info_node` into the right node's `refs` list. unsafe { info_proc.node_ref2().node.insert_node_info(info_node) }; - refs.by_node.insert(reserve1.into_node(gid, target)); - refs.by_handle.insert(reserve2.into_node(target, info_proc)); - Ok(target) + refs.by_node.insert(reserve1.into_node(gid, handle)); + by_handle_slot.insert(info_proc, reserve2); + unused_id.acquire(); + Ok(handle) } pub(crate) fn get_transaction_node(&self, handle: u32) -> BinderResult<NodeRef> { @@ -905,6 +952,16 @@ impl Process { let id = info.node_ref().node.global_id(); refs.by_handle.remove(&handle); refs.by_node.remove(&id); + refs.handle_is_present.release_id(handle as usize); + + if let Some(shrink) = refs.handle_is_present.shrink_request() { + drop(refs); + // This intentionally ignores allocation failures. + if let Ok(new_bitmap) = shrink.realloc(GFP_KERNEL) { + refs = self.node_refs.lock(); + refs.handle_is_present.shrink(new_bitmap); + } + } } } else { // All refs are cleared in process exit, so this warning is expected in that case. @@ -1260,7 +1317,7 @@ impl Process { let is_manager = { let mut inner = self.inner.lock(); inner.is_dead = true; - inner.is_frozen = false; + inner.is_frozen = IsFrozen::No; inner.sync_recv = false; inner.async_recv = false; inner.is_manager @@ -1293,7 +1350,7 @@ impl Process { { while let Some(node) = { let mut lock = self.inner.lock(); - lock.nodes.cursor_front().map(|c| c.remove_current().1) + lock.nodes.cursor_front_mut().map(|c| c.remove_current().1) } { node.to_key_value().1.release(); } @@ -1346,10 +1403,6 @@ impl Process { .alloc .take_for_each(|offset, size, debug_id, odata| { let ptr = offset + address; - pr_warn!( - "{}: removing orphan mapping {offset}:{size}\n", - self.pid_in_current_ns() - ); let mut alloc = Allocation::new(self.clone(), debug_id, offset, size, ptr, false); if let Some(data) = odata { @@ -1371,7 +1424,7 @@ impl Process { return; } inner.outstanding_txns -= 1; - inner.is_frozen && inner.outstanding_txns == 0 + inner.is_frozen.is_frozen() && inner.outstanding_txns == 0 }; if wake { @@ -1385,7 +1438,7 @@ impl Process { let mut inner = self.inner.lock(); inner.sync_recv = false; inner.async_recv = false; - inner.is_frozen = false; + inner.is_frozen = IsFrozen::No; drop(inner); msgs.send_messages(); return Ok(()); @@ -1394,7 +1447,7 @@ impl Process { let mut inner = self.inner.lock(); inner.sync_recv = false; inner.async_recv = false; - inner.is_frozen = true; + inner.is_frozen = IsFrozen::InProgress; if info.timeout_ms > 0 { let mut jiffies = kernel::time::msecs_to_jiffies(info.timeout_ms); @@ -1408,7 +1461,7 @@ impl Process { .wait_interruptible_timeout(&mut inner, jiffies) { CondVarTimeoutResult::Signal { .. } => { - inner.is_frozen = false; + inner.is_frozen = IsFrozen::No; return Err(ERESTARTSYS); } CondVarTimeoutResult::Woken { jiffies: remaining } => { @@ -1422,17 +1475,18 @@ impl Process { } if inner.txns_pending_locked() { - inner.is_frozen = false; + inner.is_frozen = IsFrozen::No; Err(EAGAIN) } else { drop(inner); match self.prepare_freeze_messages() { Ok(batch) => { + self.inner.lock().is_frozen = IsFrozen::Yes; batch.send_messages(); Ok(()) } Err(kernel::alloc::AllocError) => { - self.inner.lock().is_frozen = false; + self.inner.lock().is_frozen = IsFrozen::No; Err(ENOMEM) } } diff --git a/drivers/android/binder/range_alloc/tree.rs b/drivers/android/binder/range_alloc/tree.rs index 7b1a248fcb02..838fdd2b47ea 100644 --- a/drivers/android/binder/range_alloc/tree.rs +++ b/drivers/android/binder/range_alloc/tree.rs @@ -207,7 +207,7 @@ impl<T> TreeRangeAllocator<T> { } pub(crate) fn reservation_abort(&mut self, offset: usize) -> Result<FreedRange> { - let mut cursor = self.tree.cursor_lower_bound(&offset).ok_or_else(|| { + let mut cursor = self.tree.cursor_lower_bound_mut(&offset).ok_or_else(|| { pr_warn!( "EINVAL from range_alloc.reservation_abort - offset: {}", offset diff --git a/drivers/android/binder/rust_binderfs.c b/drivers/android/binder/rust_binderfs.c index 6b497146b698..c69026df775c 100644 --- a/drivers/android/binder/rust_binderfs.c +++ b/drivers/android/binder/rust_binderfs.c @@ -178,28 +178,17 @@ static int binderfs_binder_device_create(struct inode *ref_inode, } root = sb->s_root; - inode_lock(d_inode(root)); - - /* look it up */ - dentry = lookup_noperm(&QSTR(req->name), root); + dentry = simple_start_creating(root, req->name); if (IS_ERR(dentry)) { - inode_unlock(d_inode(root)); ret = PTR_ERR(dentry); goto err; } - if (d_really_is_positive(dentry)) { - /* already exists */ - dput(dentry); - inode_unlock(d_inode(root)); - ret = -EEXIST; - goto err; - } - inode->i_private = device; - d_instantiate(dentry, inode); + d_make_persistent(dentry, inode); + fsnotify_create(root->d_inode, dentry); - inode_unlock(d_inode(root)); + simple_done_creating(dentry); return 0; @@ -472,37 +461,9 @@ static struct inode *binderfs_make_inode(struct super_block *sb, int mode) return ret; } -static struct dentry *binderfs_create_dentry(struct dentry *parent, - const char *name) -{ - struct dentry *dentry; - - dentry = lookup_noperm(&QSTR(name), parent); - if (IS_ERR(dentry)) - return dentry; - - /* Return error if the file/dir already exists. */ - if (d_really_is_positive(dentry)) { - dput(dentry); - return ERR_PTR(-EEXIST); - } - - return dentry; -} - void rust_binderfs_remove_file(struct dentry *dentry) { - struct inode *parent_inode; - - parent_inode = d_inode(dentry->d_parent); - inode_lock(parent_inode); - if (simple_positive(dentry)) { - dget(dentry); - simple_unlink(parent_inode, dentry); - d_delete(dentry); - dput(dentry); - } - inode_unlock(parent_inode); + simple_recursive_removal(dentry, NULL); } static struct dentry *rust_binderfs_create_file(struct dentry *parent, const char *name, @@ -510,31 +471,23 @@ static struct dentry *rust_binderfs_create_file(struct dentry *parent, const cha void *data) { struct dentry *dentry; - struct inode *new_inode, *parent_inode; - struct super_block *sb; - - parent_inode = d_inode(parent); - inode_lock(parent_inode); - - dentry = binderfs_create_dentry(parent, name); - if (IS_ERR(dentry)) - goto out; - - sb = parent_inode->i_sb; - new_inode = binderfs_make_inode(sb, S_IFREG | 0444); - if (!new_inode) { - dput(dentry); - dentry = ERR_PTR(-ENOMEM); - goto out; - } + struct inode *new_inode; + new_inode = binderfs_make_inode(parent->d_sb, S_IFREG | 0444); + if (!new_inode) + return ERR_PTR(-ENOMEM); new_inode->i_fop = fops; new_inode->i_private = data; - d_instantiate(dentry, new_inode); - fsnotify_create(parent_inode, dentry); -out: - inode_unlock(parent_inode); + dentry = simple_start_creating(parent, name); + if (IS_ERR(dentry)) { + iput(new_inode); + return dentry; + } + + d_make_persistent(dentry, new_inode); + fsnotify_create(parent->d_inode, dentry); + simple_done_creating(dentry); return dentry; } @@ -556,34 +509,26 @@ static struct dentry *binderfs_create_dir(struct dentry *parent, const char *name) { struct dentry *dentry; - struct inode *new_inode, *parent_inode; - struct super_block *sb; - - parent_inode = d_inode(parent); - inode_lock(parent_inode); - - dentry = binderfs_create_dentry(parent, name); - if (IS_ERR(dentry)) - goto out; + struct inode *new_inode; - sb = parent_inode->i_sb; - new_inode = binderfs_make_inode(sb, S_IFDIR | 0755); - if (!new_inode) { - dput(dentry); - dentry = ERR_PTR(-ENOMEM); - goto out; - } + new_inode = binderfs_make_inode(parent->d_sb, S_IFDIR | 0755); + if (!new_inode) + return ERR_PTR(-ENOMEM); new_inode->i_fop = &simple_dir_operations; new_inode->i_op = &simple_dir_inode_operations; - set_nlink(new_inode, 2); - d_instantiate(dentry, new_inode); - inc_nlink(parent_inode); - fsnotify_mkdir(parent_inode, dentry); + dentry = simple_start_creating(parent, name); + if (IS_ERR(dentry)) { + iput(new_inode); + return dentry; + } -out: - inode_unlock(parent_inode); + inc_nlink(parent->d_inode); + set_nlink(new_inode, 2); + d_make_persistent(dentry, new_inode); + fsnotify_mkdir(parent->d_inode, dentry); + simple_done_creating(dentry); return dentry; } @@ -802,7 +747,7 @@ static void binderfs_kill_super(struct super_block *sb) * During inode eviction struct binderfs_info is needed. * So first wipe the super_block then free struct binderfs_info. */ - kill_litter_super(sb); + kill_anon_super(sb); if (info && info->ipc_ns) put_ipc_ns(info->ipc_ns); diff --git a/drivers/android/binder/stats.rs b/drivers/android/binder/stats.rs index a83ec111d2cb..037002651941 100644 --- a/drivers/android/binder/stats.rs +++ b/drivers/android/binder/stats.rs @@ -61,7 +61,7 @@ impl BinderStats { mod strings { use core::str::from_utf8_unchecked; - use kernel::str::CStr; + use kernel::str::{CStr, CStrExt as _}; extern "C" { static binder_command_strings: [*const u8; super::BC_COUNT]; @@ -72,7 +72,7 @@ mod strings { // SAFETY: Accessing `binder_command_strings` is always safe. let c_str_ptr = unsafe { binder_command_strings[i] }; // SAFETY: The `binder_command_strings` array only contains nul-terminated strings. - let bytes = unsafe { CStr::from_char_ptr(c_str_ptr) }.as_bytes(); + let bytes = unsafe { CStr::from_char_ptr(c_str_ptr) }.to_bytes(); // SAFETY: The `binder_command_strings` array only contains strings with ascii-chars. unsafe { from_utf8_unchecked(bytes) } } @@ -81,7 +81,7 @@ mod strings { // SAFETY: Accessing `binder_return_strings` is always safe. let c_str_ptr = unsafe { binder_return_strings[i] }; // SAFETY: The `binder_command_strings` array only contains nul-terminated strings. - let bytes = unsafe { CStr::from_char_ptr(c_str_ptr) }.as_bytes(); + let bytes = unsafe { CStr::from_char_ptr(c_str_ptr) }.to_bytes(); // SAFETY: The `binder_command_strings` array only contains strings with ascii-chars. unsafe { from_utf8_unchecked(bytes) } } diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs index 02512175d622..4bd3c0e417eb 100644 --- a/drivers/android/binder/transaction.rs +++ b/drivers/android/binder/transaction.rs @@ -249,7 +249,7 @@ impl Transaction { if oneway { if let Some(target_node) = self.target_node.clone() { - if process_inner.is_frozen { + if process_inner.is_frozen.is_frozen() { process_inner.async_recv = true; if self.flags & TF_UPDATE_TXN != 0 { if let Some(t_outdated) = @@ -270,7 +270,7 @@ impl Transaction { } } - if process_inner.is_frozen { + if process_inner.is_frozen.is_frozen() { return Err(BinderError::new_frozen_oneway()); } else { return Ok(()); @@ -280,7 +280,7 @@ impl Transaction { } } - if process_inner.is_frozen { + if process_inner.is_frozen.is_frozen() { process_inner.sync_recv = true; return Err(BinderError::new_frozen()); } |