rust: use new init API for initializing everything

This patch changes the API and internal logic such that the new init API
is used. This change results in a lot less `unsafe`, because the new API
is a safe abstraction over the unsafe interior.

Signed-off-by: Benno Lossin <y86-dev@protonmail.com>

Author: BennoLossin

drivers/android/context.rs

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use kernel::{
-    bindings,
+    bindings, new_mutex,
     prelude::*,
     security,
-    sync::{Arc, Mutex, UniqueArc},
+    sync::{Arc, Mutex},
 };
 
 use crate::{
@@ -17,7 +17,9 @@ struct Manager {
     uid: Option<bindings::kuid_t>,
 }
 
+#[pin_project]
 pub(crate) struct Context {
+    #[pin]
     manager: Mutex<Manager>,
 }
 
@@ -27,21 +29,15 @@ unsafe impl Sync for Context {}
 
 impl Context {
     pub(crate) fn new() -> Result<Arc<Self>> {
-        let mut ctx = Pin::from(UniqueArc::try_new(Self {
-            // SAFETY: Init is called below.
-            manager: unsafe {
-                Mutex::new(Manager {
+        Arc::pin_init::<core::convert::Infallible>(pin_init!(Self {
+            manager: new_mutex!(
+                Manager {
                     node: None,
                     uid: None,
-                })
-            },
-        })?);
-
-        // SAFETY: `manager` is also pinned when `ctx` is.
-        let manager = unsafe { ctx.as_mut().map_unchecked_mut(|c| &mut c.manager) };
-        kernel::mutex_init!(manager, "Context::manager");
-
-        Ok(ctx.into())
+                },
+                "Contex::manager"
+            ),
+        }))
     }
 
     pub(crate) fn set_manager_node(&self, node_ref: NodeRef) -> Result {

drivers/android/node.rs

@@ -2,8 +2,10 @@
 
 use core::sync::atomic::{AtomicU64, Ordering};
 use kernel::{
+    init::PinInit,
     io_buffer::IoBufferWriter,
     linked_list::{GetLinks, Links, List},
+    new_spinlock,
     prelude::*,
     sync::{Arc, Guard, LockedBy, Mutex, SpinLock},
     user_ptr::UserSlicePtrWriter,
@@ -54,6 +56,7 @@ struct NodeDeathInner {
     aborted: bool,
 }
 
+#[pin_project]
 pub(crate) struct NodeDeath {
     node: Arc<Node>,
     process: Arc<Process>,
@@ -63,37 +66,30 @@ pub(crate) struct NodeDeath {
     // TODO: Add the moment we're using this for two lists, which isn't safe because we want to
     // remove from the list without knowing the list it's in. We need to separate this out.
     death_links: Links<NodeDeath>,
+    #[pin]
     inner: SpinLock<NodeDeathInner>,
 }
 
 impl NodeDeath {
     /// Constructs a new node death notification object.
-    ///
-    /// # Safety
-    ///
-    /// The caller must call `NodeDeath::init` before using the notification object.
-    pub(crate) unsafe fn new(node: Arc<Node>, process: Arc<Process>, cookie: usize) -> Self {
-        Self {
+    #[allow(clippy::new_ret_no_self)]
+    pub(crate) fn new(node: Arc<Node>, process: Arc<Process>, cookie: usize) -> impl PinInit<Self> {
+        pin_init!(Self {
             node,
             process,
             cookie,
             work_links: Links::new(),
             death_links: Links::new(),
-            inner: unsafe {
-                SpinLock::new(NodeDeathInner {
+            inner: new_spinlock!(
+                NodeDeathInner {
                     dead: false,
                     cleared: false,
                     notification_done: false,
                     aborted: false,
-                })
-            },
-        }
-    }
-
-    pub(crate) fn init(self: Pin<&mut Self>) {
-        // SAFETY: `inner` is pinned when `self` is.
-        let inner = unsafe { self.map_unchecked_mut(|n| &mut n.inner) };
-        kernel::spinlock_init!(inner, "NodeDeath::inner");
+                },
+                "NodeDeath::inner"
+            ),
+        })
     }
 
     /// Sets the cleared flag to `true`.

drivers/android/process.rs

@@ -7,7 +7,7 @@ use kernel::{
     file::{self, File, IoctlCommand, IoctlHandler, PollTable},
     io_buffer::{IoBufferReader, IoBufferWriter},
     linked_list::List,
-    mm,
+    mm, new_mutex,
     pages::Pages,
     prelude::*,
     rbtree::RBTree,
@@ -241,6 +241,7 @@ impl ProcessNodeRefs {
     }
 }
 
+#[pin_project]
 pub(crate) struct Process {
     ctx: Arc<Context>,
 
@@ -254,10 +255,12 @@ pub(crate) struct Process {
     // lock. We may want to split up the process state at some point to use a spin lock for the
     // other fields.
     // TODO: Make this private again.
+    #[pin]
     pub(crate) inner: Mutex<ProcessInner>,
 
     // References are in a different mutex to avoid recursive acquisition when
     // incrementing/decrementing a node in another process.
+    #[pin]
     node_refs: Mutex<ProcessNodeRefs>,
 }
 
@@ -267,25 +270,13 @@ unsafe impl Sync for Process {}
 
 impl Process {
     fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
-        let mut process = Pin::from(UniqueArc::try_new(Self {
+        Arc::pin_init::<core::convert::Infallible>(pin_init!(Self {
             ctx,
             cred,
-            task: Task::current().group_leader().into(),
-            // SAFETY: `inner` is initialised in the call to `mutex_init` below.
-            inner: unsafe { Mutex::new(ProcessInner::new()) },
-            // SAFETY: `node_refs` is initialised in the call to `mutex_init` below.
-            node_refs: unsafe { Mutex::new(ProcessNodeRefs::new()) },
-        })?);
-
-        // SAFETY: `inner` is pinned when `Process` is.
-        let pinned = unsafe { process.as_mut().map_unchecked_mut(|p| &mut p.inner) };
-        kernel::mutex_init!(pinned, "Process::inner");
-
-        // SAFETY: `node_refs` is pinned when `Process` is.
-        let pinned = unsafe { process.as_mut().map_unchecked_mut(|p| &mut p.node_refs) };
-        kernel::mutex_init!(pinned, "Process::node_refs");
-
-        Ok(process.into())
+            task: ARef::from(Task::current().group_leader()),
+            inner: new_mutex!(ProcessInner::new(), "Process::inner"),
+            node_refs: new_mutex!(ProcessNodeRefs::new(), "Process::node_refs"),
+        }))
     }
 
     /// Attempts to fetch a work item from the process queue.
@@ -712,14 +703,15 @@ impl Process {
             return Ok(());
         }
 
-        let death = {
-            let mut pinned = Pin::from(death.write(
-                // SAFETY: `init` is called below.
-                unsafe { NodeDeath::new(info.node_ref.node.clone(), self.clone(), cookie) },
-            ));
-            pinned.as_mut().init();
-            Arc::<NodeDeath>::from(pinned)
-        };
+        let death = Arc::<NodeDeath>::from(
+            death
+                .pin_init_now::<core::convert::Infallible>(NodeDeath::new(
+                    info.node_ref.node.clone(),
+                    self.clone(),
+                    cookie,
+                ))
+                .map_err(|(i, _)| i)?,
+        );
 
         info.death = Some(death.clone());
 

drivers/android/thread.rs

@@ -10,9 +10,10 @@ use kernel::{
     file::{File, PollTable},
     io_buffer::{IoBufferReader, IoBufferWriter},
     linked_list::{GetLinks, Links, List},
+    new_condvar, new_spinlock,
     prelude::*,
     security,
-    sync::{Arc, CondVar, SpinLock, UniqueArc},
+    sync::{Arc, CondVar, SpinLock},
     user_ptr::{UserSlicePtr, UserSlicePtrWriter},
     Either,
 };
@@ -229,10 +230,13 @@ impl InnerThread {
     }
 }
 
+#[pin_project]
 pub(crate) struct Thread {
     pub(crate) id: i32,
     pub(crate) process: Arc<Process>,
+    #[pin]
     inner: SpinLock<InnerThread>,
+    #[pin]
     work_condvar: CondVar,
     links: Links<Thread>,
 }
@@ -241,31 +245,21 @@ impl Thread {
     pub(crate) fn new(id: i32, process: Arc<Process>) -> Result<Arc<Self>> {
         let return_work = Arc::try_new(ThreadError::new(InnerThread::set_return_work))?;
         let reply_work = Arc::try_new(ThreadError::new(InnerThread::set_reply_work))?;
-        let mut thread = Pin::from(UniqueArc::try_new(Self {
+        let thread = Arc::pin_init::<core::convert::Infallible>(pin_init!(Self {
             id,
             process,
-            // SAFETY: `inner` is initialised in the call to `spinlock_init` below.
-            inner: unsafe { SpinLock::new(InnerThread::new()) },
-            // SAFETY: `work_condvar` is initialised in the call to `condvar_init` below.
-            work_condvar: unsafe { CondVar::new() },
+            inner: new_spinlock!(InnerThread::new(), "Thread::inner"),
+            work_condvar: new_condvar!("Thread::work_condvar"),
             links: Links::new(),
-        })?);
-
-        // SAFETY: `inner` is pinned when `thread` is.
-        let inner = unsafe { thread.as_mut().map_unchecked_mut(|t| &mut t.inner) };
-        kernel::spinlock_init!(inner, "Thread::inner");
-
-        // SAFETY: `work_condvar` is pinned when `thread` is.
-        let condvar = unsafe { thread.as_mut().map_unchecked_mut(|t| &mut t.work_condvar) };
-        kernel::condvar_init!(condvar, "Thread::work_condvar");
+        }))?;
 
         {
             let mut inner = thread.inner.lock();
             inner.set_reply_work(reply_work);
             inner.set_return_work(return_work);
         }
 
-        Ok(thread.into())
+        Ok(thread)
     }
 
     pub(crate) fn set_current_transaction(&self, transaction: Arc<Transaction>) {

drivers/android/transaction.rs

@@ -7,6 +7,8 @@ use kernel::{
     io_buffer::IoBufferWriter,
     linked_list::List,
     linked_list::{GetLinks, Links},
+    macros::pinned_drop,
+    new_spinlock,
     prelude::*,
     sync::{Arc, SpinLock, UniqueArc},
     user_ptr::UserSlicePtrWriter,
@@ -26,7 +28,9 @@ struct TransactionInner {
     file_list: List<Box<FileInfo>>,
 }
 
+#[pin_project(PinnedDrop)]
 pub(crate) struct Transaction {
+    #[pin]
     inner: SpinLock<TransactionInner>,
     // TODO: Node should be released when the buffer is released.
     node_ref: Option<NodeRef>,
@@ -55,26 +59,20 @@ impl Transaction {
         let data_address = alloc.ptr;
         let file_list = alloc.take_file_list();
         alloc.keep_alive();
-        let mut tr = Pin::from(UniqueArc::try_new(Self {
-            // SAFETY: `spinlock_init` is called below.
-            inner: unsafe { SpinLock::new(TransactionInner { file_list }) },
+        let tr = UniqueArc::pin_init::<core::convert::Infallible>(pin_init!(Self {
+            inner: new_spinlock!(TransactionInner { file_list }, "Transaction::inner"),
             node_ref: Some(node_ref),
             stack_next,
             from: from.clone(),
             to,
             code: tr.code,
             flags: tr.flags,
-            data_size: tr.data_size as _,
+            data_size: tr.data_size as usize,
             data_address,
-            offsets_size: tr.offsets_size as _,
+            offsets_size: tr.offsets_size as usize,
             links: Links::new(),
             free_allocation: AtomicBool::new(true),
-        })?);
-
-        // SAFETY: `inner` is pinned when `tr` is.
-        let pinned = unsafe { tr.as_mut().map_unchecked_mut(|t| &mut t.inner) };
-        kernel::spinlock_init!(pinned, "Transaction::inner");
-
+        }))?;
         Ok(tr.into())
     }
 
@@ -88,26 +86,20 @@ impl Transaction {
         let data_address = alloc.ptr;
         let file_list = alloc.take_file_list();
         alloc.keep_alive();
-        let mut tr = Pin::from(UniqueArc::try_new(Self {
-            // SAFETY: `spinlock_init` is called below.
-            inner: unsafe { SpinLock::new(TransactionInner { file_list }) },
+        let tr = UniqueArc::pin_init::<core::convert::Infallible>(pin_init!(Self {
+            inner: new_spinlock!(TransactionInner { file_list }, "Transaction::inner"),
             node_ref: None,
             stack_next: None,
             from: from.clone(),
             to,
             code: tr.code,
             flags: tr.flags,
-            data_size: tr.data_size as _,
+            data_size: tr.data_size as usize,
             data_address,
-            offsets_size: tr.offsets_size as _,
+            offsets_size: tr.offsets_size as usize,
             links: Links::new(),
             free_allocation: AtomicBool::new(true),
-        })?);
-
-        // SAFETY: `inner` is pinned when `tr` is.
-        let pinned = unsafe { tr.as_mut().map_unchecked_mut(|t| &mut t.inner) };
-        kernel::spinlock_init!(pinned, "Transaction::inner");
-
+        }))?;
         Ok(tr.into())
     }
 
@@ -285,8 +277,9 @@ impl DeliverToRead for Transaction {
     }
 }
 
-impl Drop for Transaction {
-    fn drop(&mut self) {
+#[pinned_drop]
+impl PinnedDrop for Transaction {
+    fn drop(self: Pin<&mut Self>) {
         if self.free_allocation.load(Ordering::Relaxed) {
             self.to.buffer_get(self.data_address);
         }