From 02e487e3c19bc0907c8def61fd409f61dc6b0b1e Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 8 Aug 2025 13:27:06 +0000 Subject: [PATCH] ANDROID: rust_binder: don't drop AllocationInfo in reservation_commit The destructor of AllocationInfo could potentially sleep. Don't call it from reservation_commit. Reported-by: Gary Guo Fixes: dac7c66bc912 ("ANDROID: rust_binder: move Rust Binder in preparation for GKI module") Change-Id: Iea4d675c7b9db51019739c0b462b9108f7fecf0b Signed-off-by: Alice Ryhl --- drivers/android/binder/process.rs | 4 ++-- drivers/android/binder/range_alloc/array.rs | 4 ++-- drivers/android/binder/range_alloc/mod.rs | 5 ++++- drivers/android/binder/range_alloc/tree.rs | 20 ++++---------------- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index 927a7c085b1b..aa859021526d 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -1058,10 +1058,10 @@ impl Process { } } - pub(crate) fn buffer_make_freeable(&self, offset: usize, data: Option) { + pub(crate) fn buffer_make_freeable(&self, offset: usize, mut data: Option) { let mut inner = self.inner.lock(); if let Some(ref mut mapping) = &mut inner.mapping { - if mapping.alloc.reservation_commit(offset, data).is_err() { + if mapping.alloc.reservation_commit(offset, &mut data).is_err() { pr_warn!("Offset {} failed to be marked freeable\n", offset); } } diff --git a/drivers/android/binder/range_alloc/array.rs b/drivers/android/binder/range_alloc/array.rs index cd2b32c9f025..d315d1db31db 100644 --- a/drivers/android/binder/range_alloc/array.rs +++ b/drivers/android/binder/range_alloc/array.rs @@ -189,7 +189,7 @@ impl ArrayRangeAllocator { Ok(freed_range) } - pub(crate) fn reservation_commit(&mut self, offset: usize, data: Option) -> Result { + pub(crate) fn reservation_commit(&mut self, offset: usize, data: &mut Option) -> Result { // This could use a binary search, but linear scans are usually faster for small arrays. let range = self .ranges @@ -201,7 +201,7 @@ impl ArrayRangeAllocator { return Err(ENOENT); }; - range.state = DescriptorState::Allocated(reservation.clone().allocate(data)); + range.state = DescriptorState::Allocated(reservation.clone().allocate(data.take())); Ok(()) } diff --git a/drivers/android/binder/range_alloc/mod.rs b/drivers/android/binder/range_alloc/mod.rs index 829105f139bd..75c9feeb0168 100644 --- a/drivers/android/binder/range_alloc/mod.rs +++ b/drivers/android/binder/range_alloc/mod.rs @@ -239,7 +239,10 @@ impl RangeAllocator { } /// Called when an allocation is no longer in use by the kernel. - pub(crate) fn reservation_commit(&mut self, offset: usize, data: Option) -> Result { + /// + /// The value in `data` will be stored, if any. A mutable reference is used to avoid dropping + /// the `T` when an error is returned. + pub(crate) fn reservation_commit(&mut self, offset: usize, data: &mut Option) -> Result { match &mut self.inner { Impl::Empty(_size) => Err(EINVAL), Impl::Array(array) => array.reservation_commit(offset, data), diff --git a/drivers/android/binder/range_alloc/tree.rs b/drivers/android/binder/range_alloc/tree.rs index 7526f6a43d5e..7c9cd5c5727d 100644 --- a/drivers/android/binder/range_alloc/tree.rs +++ b/drivers/android/binder/range_alloc/tree.rs @@ -306,30 +306,18 @@ impl TreeRangeAllocator { Ok(freed_range) } - pub(crate) fn reservation_commit(&mut self, offset: usize, data: Option) -> Result { - let desc = self.tree.get_mut(&offset).ok_or_else(|| { - pr_warn!( - "ENOENT from range_alloc.reservation_commit - offset: {}", - offset - ); - ENOENT - })?; + pub(crate) fn reservation_commit(&mut self, offset: usize, data: &mut Option) -> Result { + let desc = self.tree.get_mut(&offset).ok_or(ENOENT)?; desc.try_change_state(|state| match state { Some((DescriptorState::Reserved(reservation), free_node_res)) => ( Some(( - DescriptorState::Allocated(reservation.allocate(data)), + DescriptorState::Allocated(reservation.allocate(data.take())), free_node_res, )), Ok(()), ), - other => { - pr_warn!( - "ENOENT from range_alloc.reservation_commit - offset: {}", - offset - ); - (other, Err(ENOENT)) - } + other => (other, Err(ENOENT)), }) }