From a1fb84ab7b92e8e9df448a79e8c02b57c98b5141 Mon Sep 17 00:00:00 2001 From: Carlos Llamas Date: Fri, 24 Oct 2025 16:15:02 +0000 Subject: binder: mark binder_alloc_exhaustive_test as slow The binder_alloc_exhaustive_test kunit test takes over 30s to complete and the kunit framework reports: # binder_alloc_exhaustive_test: Test should be marked slow (runtime: 33.842881934s) Mark the test as suggested to silence the warning. Cc: Tiffany Yang Signed-off-by: Carlos Llamas Reviewed-by: Tiffany Yang Reviewed-by: Alice Ryhl Link: https://patch.msgid.link/20251024161525.1732874-1-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/tests/binder_alloc_kunit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/android') diff --git a/drivers/android/tests/binder_alloc_kunit.c b/drivers/android/tests/binder_alloc_kunit.c index 9b884d977f76..7f9cc003bbe3 100644 --- a/drivers/android/tests/binder_alloc_kunit.c +++ b/drivers/android/tests/binder_alloc_kunit.c @@ -554,7 +554,7 @@ static void binder_alloc_test_exit(struct kunit *test) static struct kunit_case binder_alloc_test_cases[] = { KUNIT_CASE(binder_alloc_test_init_freelist), KUNIT_CASE(binder_alloc_test_mmap), - KUNIT_CASE(binder_alloc_exhaustive_test), + KUNIT_CASE_SLOW(binder_alloc_exhaustive_test), {} }; -- cgit v1.2.3 From d4b83ba11cf227705986265fab0744060c02608b Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 31 Oct 2025 08:48:18 +0000 Subject: rust_binder: use compat_ptr_ioctl Binder always treats the ioctl argument as a pointer. In this scenario, the idiomatic way to implement compat_ioctl is to use compat_ptr_ioctl. Thus update Rust Binder to do that. Signed-off-by: Alice Ryhl Acked-by: Carlos Llamas Link: https://patch.msgid.link/20251031-binder-compatptrioctl-v2-1-3d05b5cc058e@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/process.rs | 9 --------- drivers/android/binder/rust_binder_main.rs | 22 +++------------------- 2 files changed, 3 insertions(+), 28 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index 7607353a5e92..27323070f30f 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -1623,15 +1623,6 @@ impl Process { } } - pub(crate) fn compat_ioctl( - this: ArcBorrow<'_, Process>, - file: &File, - cmd: u32, - arg: usize, - ) -> Result { - Self::ioctl(this, file, cmd, arg) - } - pub(crate) fn mmap( this: ArcBorrow<'_, Process>, _file: &File, diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs index 6773b7c273ec..c79a9e742240 100644 --- a/drivers/android/binder/rust_binder_main.rs +++ b/drivers/android/binder/rust_binder_main.rs @@ -313,8 +313,8 @@ pub static rust_binder_fops: AssertSync = { let ops = kernel::bindings::file_operations { owner: THIS_MODULE.as_ptr(), poll: Some(rust_binder_poll), - unlocked_ioctl: Some(rust_binder_unlocked_ioctl), - compat_ioctl: Some(rust_binder_compat_ioctl), + unlocked_ioctl: Some(rust_binder_ioctl), + compat_ioctl: Some(bindings::compat_ptr_ioctl), mmap: Some(rust_binder_mmap), open: Some(rust_binder_open), release: Some(rust_binder_release), @@ -402,23 +402,7 @@ unsafe extern "C" fn rust_binder_release( /// # Safety /// Only called by binderfs. -unsafe extern "C" fn rust_binder_compat_ioctl( - file: *mut bindings::file, - cmd: kernel::ffi::c_uint, - arg: kernel::ffi::c_ulong, -) -> kernel::ffi::c_long { - // SAFETY: We previously set `private_data` in `rust_binder_open`. - let f = unsafe { Arc::::borrow((*file).private_data) }; - // SAFETY: The caller ensures that the file is valid. - match Process::compat_ioctl(f, unsafe { File::from_raw_file(file) }, cmd as _, arg as _) { - Ok(()) => 0, - Err(err) => err.to_errno() as isize, - } -} - -/// # Safety -/// Only called by binderfs. -unsafe extern "C" fn rust_binder_unlocked_ioctl( +unsafe extern "C" fn rust_binder_ioctl( file: *mut bindings::file, cmd: kernel::ffi::c_uint, arg: kernel::ffi::c_ulong, -- cgit v1.2.3 From c1437332e4d351e86049c275d879110f4dabe7b7 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Wed, 29 Oct 2025 11:50:58 +0000 Subject: rust_binder: move BC_FREE_BUFFER drop inside if statement When looking at flamegraphs, there is a pretty large entry for the function call drop_in_place::> which in turn calls drop_in_place::. Combined with the looper_need_return condition, this means that the generated code looks like this: if let Some(buffer) = buffer { if buffer.looper_need_return_on_free() { self.inner.lock().looper_need_return = true; } } drop_in_place::>() { // not inlined if let Some(buffer) = buffer { drop_in_place::(buffer); } } This kind of situation where you check X and then check X again is normally optimized into a single condition, but in this case due to the non-inlined function call to drop_in_place::>, that optimization does not happen. Furthermore, the drop_in_place:: call is only two-thirds of the drop_in_place::> call in the flamegraph. This indicates that this double condition is not performing well. Also, last time I looked at Binder perf, I remember finding that the destructor of Allocation was involved with many branch mispredictions. Thus, change this code to look like this: if let Some(buffer) = buffer { if buffer.looper_need_return_on_free() { self.inner.lock().looper_need_return = true; } drop_in_place::(buffer); } by dropping the Allocation directly. Flamegraphs confirm that the drop_in_place::> call disappears from this change. Signed-off-by: Alice Ryhl Acked-by: Carlos Llamas Link: https://patch.msgid.link/20251029-binder-bcfreebuf-option-v1-1-4d282be0439f@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/thread.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs index 7e34ccd394f8..1a8e6fdc0dc4 100644 --- a/drivers/android/binder/thread.rs +++ b/drivers/android/binder/thread.rs @@ -1323,12 +1323,12 @@ impl Thread { } BC_FREE_BUFFER => { let buffer = self.process.buffer_get(reader.read()?); - if let Some(buffer) = &buffer { + if let Some(buffer) = buffer { if buffer.looper_need_return_on_free() { self.inner.lock().looper_need_return = true; } + drop(buffer); } - drop(buffer); } BC_INCREFS => { self.process -- cgit v1.2.3 From 77198581e0d05aae08a06f471e21a19ab0edaea2 Mon Sep 17 00:00:00 2001 From: Sunday Adelodun Date: Fri, 21 Nov 2025 12:12:02 +0100 Subject: android: binderfs: add missing parameters in binder_ctl_ioctl()'s doc The kernel-doc comment for binder_ctl_ioctl() lacks descriptions for the @file, @cmd, and @arg parameters, which triggers warnings during documentation builds. Add the missing parameter descriptions to keep the kernel-doc consistent and free of warnings. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202511201725.ni2HZ2PP-lkp@intel.com/ Signed-off-by: Sunday Adelodun Link: https://patch.msgid.link/20251121111203.21800-1-adelodunolaoluwa@yahoo.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binderfs.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/android') diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c index be8e64eb39ec..47f6d1e5971e 100644 --- a/drivers/android/binderfs.c +++ b/drivers/android/binderfs.c @@ -224,6 +224,9 @@ err: /** * binder_ctl_ioctl - handle binder device node allocation requests + * @file: The file pointer for the binder-control device node. + * @cmd: The ioctl command. + * @arg: The ioctl argument. * * The request handler for the binder-control device. All requests operate on * the binderfs mount the binder-control device resides in: -- cgit v1.2.3 From 1e9a37d35a0ea658df7b5d64889ec2bd529f46d6 Mon Sep 17 00:00:00 2001 From: Sunday Adelodun Date: Fri, 21 Nov 2025 12:12:03 +0100 Subject: android: binder: add missing return value documentation for binder_apply_fd_fixups() The kernel-doc for binder_apply_fd_fixups() was missing a description of its return value, which triggers a kernel-doc warning. Add the missing "Return:" entry to doc that the function returns 0 on success or a negative errno on failure. Signed-off-by: Sunday Adelodun Link: https://patch.msgid.link/20251121111203.21800-2-adelodunolaoluwa@yahoo.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/android') diff --git a/drivers/android/binder.c b/drivers/android/binder.c index a3a1b5c33ba3..535fc881c8da 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -4669,6 +4669,8 @@ static int binder_wait_for_work(struct binder_thread *thread, * * If we fail to allocate an fd, skip the install and release * any fds that have already been allocated. + * + * Return: 0 on success, a negative errno code on failure. */ static int binder_apply_fd_fixups(struct binder_proc *proc, struct binder_transaction *t) -- cgit v1.2.3 From 3e0ae02ba831da2b707905f4e602e43f8507b8cc Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 11 Nov 2025 14:23:32 +0000 Subject: rust_binder: fix race condition on death_list Rust Binder contains the following unsafe operation: // SAFETY: A `NodeDeath` is never inserted into the death list // of any node other than its owner, so it is either in this // death list or in no death list. unsafe { node_inner.death_list.remove(self) }; This operation is unsafe because when touching the prev/next pointers of a list element, we have to ensure that no other thread is also touching them in parallel. If the node is present in the list that `remove` is called on, then that is fine because we have exclusive access to that list. If the node is not in any list, then it's also ok. But if it's present in a different list that may be accessed in parallel, then that may be a data race on the prev/next pointers. And unfortunately that is exactly what is happening here. In Node::release, we: 1. Take the lock. 2. Move all items to a local list on the stack. 3. Drop the lock. 4. Iterate the local list on the stack. Combined with threads using the unsafe remove method on the original list, this leads to memory corruption of the prev/next pointers. This leads to crashes like this one: Unable to handle kernel paging request at virtual address 000bb9841bcac70e Mem abort info: ESR = 0x0000000096000044 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x04: level 0 translation fault Data abort info: ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000 CM = 0, WnR = 1, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [000bb9841bcac70e] address between user and kernel address ranges Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP google-cdd 538c004.gcdd: context saved(CPU:1) item - log_kevents is disabled Modules linked in: ... rust_binder CPU: 1 UID: 0 PID: 2092 Comm: kworker/1:178 Tainted: G S W OE 6.12.52-android16-5-g98debd5df505-4k #1 f94a6367396c5488d635708e43ee0c888d230b0b Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE Hardware name: MUSTANG PVT 1.0 based on LGA (DT) Workqueue: events _RNvXs6_NtCsdfZWD8DztAw_6kernel9workqueueINtNtNtB7_4sync3arc3ArcNtNtCs8QPsHWIn21X_16rust_binder_main7process7ProcessEINtB5_15WorkItemPointerKy0_E3runB13_ [rust_binder] pstate: 23400005 (nzCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--) pc : _RNvXs3_NtCs8QPsHWIn21X_16rust_binder_main7processNtB5_7ProcessNtNtCsdfZWD8DztAw_6kernel9workqueue8WorkItem3run+0x450/0x11f8 [rust_binder] lr : _RNvXs3_NtCs8QPsHWIn21X_16rust_binder_main7processNtB5_7ProcessNtNtCsdfZWD8DztAw_6kernel9workqueue8WorkItem3run+0x464/0x11f8 [rust_binder] sp : ffffffc09b433ac0 x29: ffffffc09b433d30 x28: ffffff8821690000 x27: ffffffd40cbaa448 x26: ffffff8821690000 x25: 00000000ffffffff x24: ffffff88d0376578 x23: 0000000000000001 x22: ffffffc09b433c78 x21: ffffff88e8f9bf40 x20: ffffff88e8f9bf40 x19: ffffff882692b000 x18: ffffffd40f10bf00 x17: 00000000c006287d x16: 00000000c006287d x15: 00000000000003b0 x14: 0000000000000100 x13: 000000201cb79ae0 x12: fffffffffffffff0 x11: 0000000000000000 x10: 0000000000000001 x9 : 0000000000000000 x8 : b80bb9841bcac706 x7 : 0000000000000001 x6 : fffffffebee63f30 x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000000 x2 : 0000000000004c31 x1 : ffffff88216900c0 x0 : ffffff88e8f9bf00 Call trace: _RNvXs3_NtCs8QPsHWIn21X_16rust_binder_main7processNtB5_7ProcessNtNtCsdfZWD8DztAw_6kernel9workqueue8WorkItem3run+0x450/0x11f8 [rust_binder bbc172b53665bbc815363b22e97e3f7e3fe971fc] process_scheduled_works+0x1c4/0x45c worker_thread+0x32c/0x3e8 kthread+0x11c/0x1c8 ret_from_fork+0x10/0x20 Code: 94218d85 b4000155 a94026a8 d10102a0 (f9000509) ---[ end trace 0000000000000000 ]--- Thus, modify Node::release to pop items directly off the original list. Cc: stable@vger.kernel.org Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") Signed-off-by: Alice Ryhl Acked-by: Miguel Ojeda Link: https://patch.msgid.link/20251111-binder-fix-list-remove-v1-1-8ed14a0da63d@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/node.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/node.rs b/drivers/android/binder/node.rs index 08d362deaf61..c26d113ede96 100644 --- a/drivers/android/binder/node.rs +++ b/drivers/android/binder/node.rs @@ -541,10 +541,10 @@ impl Node { guard = self.owner.inner.lock(); } - let death_list = core::mem::take(&mut self.inner.access_mut(&mut guard).death_list); - drop(guard); - for death in death_list { + while let Some(death) = self.inner.access_mut(&mut guard).death_list.pop_front() { + drop(guard); death.into_arc().set_dead(); + guard = self.owner.inner.lock(); } } -- cgit v1.2.3 From 6c37bebd8c926ad01ef157c0d123633a203e5c0d Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 11 Nov 2025 14:23:33 +0000 Subject: rust_binder: avoid mem::take on delivered_deaths Similar to the previous commit, List::remove is used on delivered_deaths, so do not use mem::take on it as that may result in violations of the List::remove safety requirements. I don't think this particular case can be triggered because it requires fd close to run in parallel with an ioctl on the same fd. But let's not tempt fate. Cc: stable@vger.kernel.org Fixes: eafedbc7c050 ("rust_binder: add Rust Binder driver") Signed-off-by: Alice Ryhl Acked-by: Miguel Ojeda Link: https://patch.msgid.link/20251111-binder-fix-list-remove-v1-2-8ed14a0da63d@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder/process.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/android') diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index 27323070f30f..fd5dcdc8788c 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -1362,8 +1362,12 @@ impl Process { work.into_arc().cancel(); } - let delivered_deaths = take(&mut self.inner.lock().delivered_deaths); - drop(delivered_deaths); + // Clear delivered_deaths list. + // + // Scope ensures that MutexGuard is dropped while executing the body. + while let Some(delivered_death) = { self.inner.lock().delivered_deaths.pop_front() } { + drop(delivered_death); + } // Free any resources kept alive by allocated buffers. let omapping = self.inner.lock().mapping.take(); -- cgit v1.2.3