lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260119133101.23708-1-jason.kei.hall@gmail.com>
Date: Mon, 19 Jan 2026 06:31:01 -0700
From: Jason Hall <jason.kei.hall@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Arve Hjønnevåg <arve@...roid.com>,
	Todd Kjos <tkjos@...gle.com>,
	Carlos Llamas <cmllamas@...gle.com>,
	Jason Hall <jason.kei.hall@...il.com>
Subject: [PATCH v3] rust_binder: refactor context management to use KVVec

Replace the linked list management in context.rs with KVVec.
This simplifies the ownership model by using standard
Arc-based tracking and moves away from manual unsafe list removals.

The refactor improves memory safety by leveraging Rust's contiguous
collection types while maintaining proper error propagation for
allocation failures during process registration.

Suggested-by: Alice Ryhl <aliceryhl@...gle.com>
Link: https://github.com/rust-for-linux/linux/issues/1215
Signed-off-by: Jason Hall <jason.kei.hall@...il.com>
---
v3:
  - Simplified register_process using ?.
  - Used Arc::ptr_eq for comparisons in deregister_process.
  - Changed deregister_process to take &Arc<Process>.
  - Added capacity shrinking logic to Context's process list.
  - Optimized get_procs_with_pid to iterate directly under the lock.
  - Removed ListArc usage in process.rs.
---
 drivers/android/binder/context.rs | 91 ++++++++++++++-----------------
 drivers/android/binder/process.rs |  5 +-
 2 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
index 3d135ec03ca7..dedcfd9dfea2 100644
--- a/drivers/android/binder/context.rs
+++ b/drivers/android/binder/context.rs
@@ -3,8 +3,8 @@
 // Copyright (C) 2025 Google LLC.
 
 use kernel::{
-    error::Error,
-    list::{List, ListArc, ListLinks},
+    alloc::kvec::KVVec,
+    error::code::*,
     prelude::*,
     security,
     str::{CStr, CString},
@@ -17,22 +17,19 @@
 kernel::sync::global_lock! {
     // SAFETY: We call `init` in the module initializer, so it's initialized before first use.
     pub(crate) unsafe(uninit) static CONTEXTS: Mutex<ContextList> = ContextList {
-        list: List::new(),
+        contexts: KVVec::new(),
     };
 }
 
 pub(crate) struct ContextList {
-    list: List<Context>,
+    contexts: KVVec<Arc<Context>>,
 }
 
-pub(crate) fn get_all_contexts() -> Result<KVec<Arc<Context>>> {
+pub(crate) fn get_all_contexts() -> Result<KVVec<Arc<Context>>> {
     let lock = CONTEXTS.lock();
-
-    let count = lock.list.iter().count();
-
-    let mut ctxs = KVec::with_capacity(count, GFP_KERNEL)?;
-    for ctx in &lock.list {
-        ctxs.push(Arc::from(ctx), GFP_KERNEL)?;
+    let mut ctxs = KVVec::with_capacity(lock.contexts.len(), GFP_KERNEL)?;
+    for ctx in lock.contexts.iter() {
+        ctxs.push(ctx.clone(), GFP_KERNEL)?;
     }
     Ok(ctxs)
 }
@@ -42,7 +39,7 @@ pub(crate) fn get_all_contexts() -> Result<KVec<Arc<Context>>> {
 struct Manager {
     node: Option<NodeRef>,
     uid: Option<Kuid>,
-    all_procs: List<Process>,
+    all_procs: KVVec<Arc<Process>>,
 }
 
 /// There is one context per binder file (/dev/binder, /dev/hwbinder, etc)
@@ -51,28 +48,16 @@ pub(crate) struct Context {
     #[pin]
     manager: Mutex<Manager>,
     pub(crate) name: CString,
-    #[pin]
-    links: ListLinks,
-}
-
-kernel::list::impl_list_arc_safe! {
-    impl ListArcSafe<0> for Context { untracked; }
-}
-kernel::list::impl_list_item! {
-    impl ListItem<0> for Context {
-        using ListLinks { self.links };
-    }
 }
 
 impl Context {
     pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
         let name = CString::try_from(name)?;
-        let list_ctx = ListArc::pin_init::<Error>(
+        let ctx = Arc::pin_init(
             try_pin_init!(Context {
                 name,
-                links <- ListLinks::new(),
                 manager <- kernel::new_mutex!(Manager {
-                    all_procs: List::new(),
+                    all_procs: KVVec::new(),
                     node: None,
                     uid: None,
                 }, "Context::manager"),
@@ -80,8 +65,7 @@ pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
             GFP_KERNEL,
         )?;
 
-        let ctx = list_ctx.clone_arc();
-        CONTEXTS.lock().list.push_back(list_ctx);
+        CONTEXTS.lock().contexts.push(ctx.clone(), GFP_KERNEL)?;
 
         Ok(ctx)
     }
@@ -90,26 +74,35 @@ pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
     ///
     /// No-op if called twice.
     pub(crate) fn deregister(&self) {
-        // SAFETY: We never add the context to any other linked list than this one, so it is either
-        // in this list, or not in any list.
-        unsafe { CONTEXTS.lock().list.remove(self) };
+        // Safe removal using retain
+        CONTEXTS.lock().contexts.retain(|c| {
+            let p1 = Arc::as_ptr(c);
+            let p2 = self as *const Context;
+            p1 != p2
+        });
     }
 
-    pub(crate) fn register_process(self: &Arc<Self>, proc: ListArc<Process>) {
+    pub(crate) fn register_process(self: &Arc<Self>, proc: Arc<Process>) -> Result {
         if !Arc::ptr_eq(self, &proc.ctx) {
             pr_err!("Context::register_process called on the wrong context.");
-            return;
+            return Err(EINVAL);
         }
-        self.manager.lock().all_procs.push_back(proc);
+        self.manager.lock().all_procs.push(proc, GFP_KERNEL)?;
+        Ok(())
     }
 
-    pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Process) {
+    pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Arc<Process>) {
         if !Arc::ptr_eq(self, &proc.ctx) {
             pr_err!("Context::deregister_process called on the wrong context.");
             return;
         }
-        // SAFETY: We just checked that this is the right list.
-        unsafe { self.manager.lock().all_procs.remove(proc) };
+        let mut manager = self.manager.lock();
+        manager.all_procs.retain(|p| !Arc::ptr_eq(p, proc));
+        let len = manager.all_procs.len();
+        let cap = manager.all_procs.capacity();
+        if len < cap / 2 && cap > 128 {
+            let _ = manager.all_procs.reserve(0, GFP_KERNEL);
+        }
     }
 
     pub(crate) fn set_manager_node(&self, node_ref: NodeRef) -> Result {
@@ -158,23 +151,23 @@ pub(crate) fn for_each_proc<F>(&self, mut func: F)
         }
     }
 
-    pub(crate) fn get_all_procs(&self) -> Result<KVec<Arc<Process>>> {
+    pub(crate) fn get_all_procs(&self) -> Result<KVVec<Arc<Process>>> {
         let lock = self.manager.lock();
-        let count = lock.all_procs.iter().count();
-
-        let mut procs = KVec::with_capacity(count, GFP_KERNEL)?;
-        for proc in &lock.all_procs {
-            procs.push(Arc::from(proc), GFP_KERNEL)?;
+        let mut procs = KVVec::with_capacity(lock.all_procs.len(), GFP_KERNEL)?;
+        for proc in lock.all_procs.iter() {
+            procs.push(Arc::clone(proc), GFP_KERNEL)?;
         }
         Ok(procs)
     }
 
-    pub(crate) fn get_procs_with_pid(&self, pid: i32) -> Result<KVec<Arc<Process>>> {
-        let orig = self.get_all_procs()?;
-        let mut backing = KVec::with_capacity(orig.len(), GFP_KERNEL)?;
-        for proc in orig.into_iter().filter(|proc| proc.task.pid() == pid) {
-            backing.push(proc, GFP_KERNEL)?;
+    pub(crate) fn get_procs_with_pid(&self, pid: i32) -> Result<KVVec<Arc<Process>>> {
+        let lock = self.manager.lock();
+        let mut matching_procs = KVVec::new();
+        for proc in lock.all_procs.iter() {
+            if proc.task.pid() == pid {
+                matching_procs.push(Arc::clone(proc), GFP_KERNEL)?;
+            }
         }
-        Ok(backing)
+        Ok(matching_procs)
     }
 }
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 132055b4790f..286d1badf19b 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -496,7 +496,7 @@ fn run(me: Arc<Self>) {
 impl Process {
     fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
         let current = kernel::current!();
-        let list_process = ListArc::pin_init::<Error>(
+        let process = Arc::pin_init::<Error>(
             try_pin_init!(Process {
                 ctx,
                 cred,
@@ -512,8 +512,7 @@ fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
             GFP_KERNEL,
         )?;
 
-        let process = list_process.clone_arc();
-        process.ctx.register_process(list_process);
+        process.ctx.register_process(process.clone())?;
 
         Ok(process)
     }
-- 
2.43.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ