Skip to content

Commit bc4a982

Browse files
Darksonngregkh
authored andcommitted
rust_binder: clear freeze listener on node removal
Generally userspace is supposed to explicitly clear freeze listeners before they drop the refcount on the node ref to zero, but there's nothing forcing that. Currently, in this scenario the freeze listener remains in the freeze_listeners rbtree and in the remote node's freeze listener list, even though the ref for which the listener is registered is gone. This could potentially lead to a memory leak due to a refcount cycle. Thus, remove the freeze listener in this scenario. Cc: stable <stable@kernel.org> Fixes: eafedbc ("rust_binder: add Rust Binder driver") Signed-off-by: Alice Ryhl <aliceryhl@google.com> Link: https://patch.msgid.link/20260703-remove-freeze-on-remove-node-v3-1-6e0c4547af46@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 6849cab commit bc4a982

3 files changed

Lines changed: 26 additions & 7 deletions

File tree

drivers/android/binder/freeze.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,17 @@ impl DeliverToRead for FreezeMessage {
154154
}
155155

156156
impl FreezeListener {
157-
pub(crate) fn on_process_exit(&self, proc: &Arc<Process>) {
157+
/// Called when this freeze listener is cleared abnormally.
158+
///
159+
/// This occurs either because the process exited or because the process dropped its last
160+
/// refcount on the node ref without explicitly removing the freeze listener first.
161+
///
162+
/// The returned `KVVec` is just a value that should be dropped outside of the lock.
163+
pub(crate) fn on_process_cleanup(&self, proc: &Process) -> KVVec<Arc<Process>> {
158164
if !self.is_clearing {
159-
self.node.remove_freeze_listener(proc);
165+
return self.node.remove_freeze_listener(proc);
160166
}
167+
KVVec::new()
161168
}
162169
}
163170

drivers/android/binder/node.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -682,21 +682,23 @@ impl Node {
682682
}
683683
}
684684

685-
pub(crate) fn remove_freeze_listener(&self, p: &Arc<Process>) {
686-
let _unused_capacity;
685+
pub(crate) fn remove_freeze_listener(&self, p: &Process) -> KVVec<Arc<Process>> {
687686
let mut guard = self.owner.inner.lock();
688687
let inner = self.inner.access_mut(&mut guard);
689688
let len = inner.freeze_list.len();
690-
inner.freeze_list.retain(|proc| !Arc::ptr_eq(proc, p));
689+
inner
690+
.freeze_list
691+
.retain(|proc| !core::ptr::eq::<Process>(&**proc, p));
691692
if len == inner.freeze_list.len() {
692693
pr_warn!(
693694
"Could not remove freeze listener for {}\n",
694695
p.pid_in_current_ns()
695696
);
696697
}
697698
if inner.freeze_list.is_empty() {
698-
_unused_capacity = mem::take(&mut inner.freeze_list);
699+
return mem::take(&mut inner.freeze_list);
699700
}
701+
KVVec::new()
700702
}
701703

702704
pub(crate) fn freeze_list<'a>(&'a self, guard: &'a ProcessInner) -> &'a [Arc<Process>] {

drivers/android/binder/process.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,8 @@ impl Process {
946946

947947
// To preserve original binder behaviour, we only fail requests where the manager tries to
948948
// increment references on itself.
949+
let _to_free_freeze_listener;
950+
let _to_free_freeze_listener_cleanup;
949951
let mut refs = self.node_refs.lock();
950952
if let Some(info) = refs.by_handle.get_mut(&handle) {
951953
if info.node_ref().update(inc, strong) {
@@ -961,6 +963,14 @@ impl Process {
961963
unsafe { info.node_ref2().node.remove_node_info(info) };
962964

963965
let id = info.node_ref().node.global_id();
966+
967+
if let Some(freeze) = *info.freeze() {
968+
if let Some(fl) = refs.freeze_listeners.remove(&freeze) {
969+
_to_free_freeze_listener_cleanup = fl.on_process_cleanup(&self);
970+
_to_free_freeze_listener = fl;
971+
}
972+
}
973+
964974
refs.by_handle.remove(&handle);
965975
refs.by_node.remove(&id);
966976
refs.handle_is_present.release_id(handle as usize);
@@ -1384,7 +1394,7 @@ impl Process {
13841394
// Clean up freeze listeners.
13851395
let freeze_listeners = take(&mut self.node_refs.lock().freeze_listeners);
13861396
for listener in freeze_listeners.values() {
1387-
listener.on_process_exit(&self);
1397+
listener.on_process_cleanup(&self);
13881398
}
13891399
drop(freeze_listeners);
13901400

0 commit comments

Comments
 (0)