rust_binder: avoid allocating under node_refs for freeze listeners

The node_refs mutex needs to be changed to a spinlock, so in preparation
for that, update freeze.rs to avoid allocating under the node_refs lock.
This is done by adding a retry loop so that if add_freeze_listener()
requires reallocating the KVVec<_> of freeze listeners, the caller will
allocate a larger vector and retry.

Analogously, the remove_freeze_listener() function is updated to return
the empty KVVec<_> when it is no longer needed, to avoid calling
kvfree() under the node_refs lock.

Reviewed-by: Matthew Maurer <mmaurer@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Link: https://patch.msgid.link/20260615-binder-noderefs-spin-v3-1-3235f5a3e0a0@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Alice Ryhl
2026-06-15 12:36:41 +00:00
committed by Greg Kroah-Hartman
parent 9e32d2a978
commit b9d17aa74d
2 changed files with 67 additions and 45 deletions

View File

@@ -173,36 +173,58 @@ pub(crate) fn request_freeze_notif(
let msg = FreezeMessage::new(GFP_KERNEL)?;
let alloc = RBTreeNodeReservation::new(GFP_KERNEL)?;
let mut afl_vec_alloc = KVVec::new();
let mut info;
let mut freeze_entry;
let mut node_refs_guard = self.node_refs.lock();
let node_refs = &mut *node_refs_guard;
let Some(info) = node_refs.by_handle.get_mut(&handle) else {
pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION invalid ref {}\n", handle);
return Err(EINVAL);
};
if info.freeze().is_some() {
pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION already set\n");
return Err(EINVAL);
}
let node_ref = info.node_ref();
let freeze_entry = node_refs.freeze_listeners.entry(cookie);
if let rbtree::Entry::Occupied(ref dupe) = freeze_entry {
if !dupe.get().allow_duplicate(&node_ref.node) {
pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION duplicate cookie\n");
loop {
let node_refs = &mut *node_refs_guard;
info = match node_refs.by_handle.get_mut(&handle) {
Some(info) => info,
None => {
pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION invalid ref {}\n", handle);
return Err(EINVAL);
}
};
if info.freeze().is_some() {
pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION already set\n");
return Err(EINVAL);
}
}
let node_ref = info.node_ref();
freeze_entry = node_refs.freeze_listeners.entry(cookie);
// All failure paths must come before this call, and all modifications must come after this
// call.
node_ref.node.add_freeze_listener(self, GFP_KERNEL)?;
if let rbtree::Entry::Occupied(ref dupe) = freeze_entry {
if !dupe.get().allow_duplicate(&node_ref.node) {
pr_warn!("BC_REQUEST_FREEZE_NOTIFICATION duplicate cookie\n");
return Err(EINVAL);
}
}
// Now we add to the node's freeze listener list, with retry and re-allocate if the
// vector is full.
//
// To ensure that the node is added atomically, this is the first time we modify any
// state. When this call succeeds, all other modifications must occur without the
// possibility for any failure paths.
match node_ref
.node
.add_freeze_listener(self, &mut afl_vec_alloc)?
{
Ok(()) => break,
Err(resize_target) => {
drop(node_refs_guard);
afl_vec_alloc = KVVec::with_capacity(resize_target, GFP_KERNEL)?;
node_refs_guard = self.node_refs.lock();
}
}
}
match freeze_entry {
rbtree::Entry::Vacant(entry) => {
entry.insert(
FreezeListener {
cookie,
node: node_ref.node.clone(),
node: info.node_ref().node.clone(),
last_is_frozen: None,
is_pending: false,
is_clearing: false,
@@ -273,6 +295,7 @@ pub(crate) fn clear_freeze_notif(self: &Arc<Self>, reader: &mut UserSliceReader)
let handle = hc.handle;
let cookie = FreezeCookie(hc.cookie);
let _to_free_fl;
let alloc = FreezeMessage::new(GFP_KERNEL)?;
let mut node_refs_guard = self.node_refs.lock();
let node_refs = &mut *node_refs_guard;
@@ -293,7 +316,7 @@ pub(crate) fn clear_freeze_notif(self: &Arc<Self>, reader: &mut UserSliceReader)
return Err(EINVAL);
};
listener.is_clearing = true;
listener.node.remove_freeze_listener(self);
_to_free_fl = listener.node.remove_freeze_listener(self);
*info.freeze() = None;
let mut msg = None;
if !listener.is_pending {

View File

@@ -659,33 +659,29 @@ fn do_work_locked(
pub(crate) fn add_freeze_listener(
&self,
process: &Arc<Process>,
flags: kernel::alloc::Flags,
) -> Result {
let mut vec_alloc = KVVec::<Arc<Process>>::new();
loop {
let mut guard = self.owner.inner.lock();
// Do not check for `guard.dead`. The `dead` flag that matters here is the owner of the
// listener, no the target.
let inner = self.inner.access_mut(&mut guard);
let len = inner.freeze_list.len();
if len >= inner.freeze_list.capacity() {
if len >= vec_alloc.capacity() {
drop(guard);
vec_alloc = KVVec::with_capacity((1 + len).next_power_of_two(), flags)?;
continue;
}
mem::swap(&mut inner.freeze_list, &mut vec_alloc);
for elem in vec_alloc.drain_all() {
inner.freeze_list.push_within_capacity(elem)?;
}
// If the vector needs to be resized, it's done via this argument.
vec_alloc: &mut KVVec<Arc<Process>>,
) -> Result<Result<(), usize>> {
let mut guard = self.owner.inner.lock();
// Do not check for `guard.dead`. The `dead` flag that matters here is the owner of the
// listener, not the target.
let inner = self.inner.access_mut(&mut guard);
let len = inner.freeze_list.len();
if len == inner.freeze_list.capacity() {
if len >= vec_alloc.capacity() {
// Request the caller to reallocate.
return Ok(Err((1 + len).next_power_of_two()));
}
mem::swap(&mut inner.freeze_list, vec_alloc);
for elem in vec_alloc.drain_all() {
inner.freeze_list.push_within_capacity(elem)?;
}
inner.freeze_list.push_within_capacity(process.clone())?;
return Ok(());
}
inner.freeze_list.push_within_capacity(process.clone())?;
Ok(Ok(()))
}
pub(crate) fn remove_freeze_listener(&self, p: &Arc<Process>) {
let _unused_capacity;
pub(crate) fn remove_freeze_listener(&self, p: &Arc<Process>) -> KVVec<Arc<Process>> {
let mut guard = self.owner.inner.lock();
let inner = self.inner.access_mut(&mut guard);
let len = inner.freeze_list.len();
@@ -696,9 +692,12 @@ pub(crate) fn remove_freeze_listener(&self, p: &Arc<Process>) {
p.pid_in_current_ns()
);
}
// If the vector is empty it needs to be freed. However, we can't free it here because that
// might sleep, so return it to the caller.
if inner.freeze_list.is_empty() {
_unused_capacity = mem::take(&mut inner.freeze_list);
return mem::take(&mut inner.freeze_list);
}
KVVec::new()
}
pub(crate) fn freeze_list<'a>(&'a self, guard: &'a ProcessInner) -> &'a [Arc<Process>] {