rust: only require context when registering a miscdev device

This way it's possible to create a registration instance without having
the context yet, and later provide the context when actually
registering. This is useful for cases when the registration is
contained in the context data, for example, when registering a new
`miscdev` from `probe` functions.

Signed-off-by: Wedson Almeida Filho <[email protected]>

Author: wedsonaf

rust/kernel/lib.rs

@@ -20,6 +20,7 @@
     const_mut_refs,
     doc_cfg,
     generic_associated_types,
+    maybe_uninit_extra,
     ptr_metadata,
     receiver_trait,
     coerce_unsized,

rust/kernel/miscdev.rs

@@ -12,29 +12,34 @@ use crate::file_operations::{FileOpenAdapter, FileOpener, FileOperationsVtable};
 use crate::{str::CStr, KernelModule, ThisModule};
 use alloc::boxed::Box;
 use core::marker::{PhantomData, PhantomPinned};
-use core::pin::Pin;
+use core::{mem::MaybeUninit, pin::Pin};
 
 /// A registration of a miscellaneous device.
+///
+/// # Invariants
+///
+/// `Context` is always initialised when `registered` is `true`, and not initialised otherwise.
 pub struct Registration<T: Sync = ()> {
     registered: bool,
     mdev: bindings::miscdevice,
     _pin: PhantomPinned,
 
     /// Context initialised on construction and made available to all file instances on
     /// [`FileOpener::open`].
-    pub context: T,
+    open_data: MaybeUninit<T>,
 }
 
 impl<T: Sync> Registration<T> {
     /// Creates a new [`Registration`] but does not register it yet.
     ///
     /// It is allowed to move.
-    pub fn new(context: T) -> Self {
+    pub fn new() -> Self {
+        // INVARIANT: `registered` is `false` and `open_data` is not initialised.
         Self {
             registered: false,
             mdev: bindings::miscdevice::default(),
             _pin: PhantomPinned,
-            context,
+            open_data: MaybeUninit::uninit(),
         }
     }
 
@@ -44,10 +49,10 @@ impl<T: Sync> Registration<T> {
     pub fn new_pinned<F: FileOpener<T>>(
         name: &'static CStr,
         minor: Option<i32>,
-        context: T,
+        open_data: T,
     ) -> Result<Pin<Box<Self>>> {
-        let mut r = Pin::from(Box::try_new(Self::new(context))?);
-        r.as_mut().register::<F>(name, minor)?;
+        let mut r = Pin::from(Box::try_new(Self::new())?);
+        r.as_mut().register::<F>(name, minor, open_data)?;
         Ok(r)
     }
 
@@ -59,6 +64,7 @@ impl<T: Sync> Registration<T> {
         self: Pin<&mut Self>,
         name: &'static CStr,
         minor: Option<i32>,
+        open_data: T,
     ) -> Result {
         // SAFETY: We must ensure that we never move out of `this`.
         let this = unsafe { self.get_unchecked_mut() };
@@ -72,31 +78,51 @@ impl<T: Sync> Registration<T> {
         this.mdev.name = name.as_char_ptr();
         this.mdev.minor = minor.unwrap_or(bindings::MISC_DYNAMIC_MINOR as i32);
 
+        // We write to `open_data` here because as soon as `misc_register` succeeds, the file can be
+        // opened, so we need `open_data` configured ahead of time.
+        //
+        // INVARIANT: `registered` is set to `true`, but `open_data` is also initialised.
+        this.registered = true;
+        this.open_data.write(open_data);
+
         let ret = unsafe { bindings::misc_register(&mut this.mdev) };
         if ret < 0 {
+            // INVARIANT: `registered` is set back to `false` and the `open_data` is destructued.
+            this.registered = false;
+            // SAFETY: `open_data` was initialised a few lines above.
+            unsafe { this.open_data.assume_init_drop() };
             return Err(Error::from_kernel_errno(ret));
         }
-        this.registered = true;
+
         Ok(())
     }
 }
 
+impl<T: Sync> Default for Registration<T> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl<T: Sync> FileOpenAdapter for Registration<T> {
     type Arg = T;
 
     unsafe fn convert(_inode: *mut bindings::inode, file: *mut bindings::file) -> *const Self::Arg {
         // SAFETY: the caller must guarantee that `file` is valid.
         let reg = crate::container_of!(unsafe { (*file).private_data }, Self, mdev);
-        unsafe { &(*reg).context }
+
+        // SAFETY: This function is only called while the misc device is still registered, so the
+        // registration must be valid. Additionally, the type invariants guarantee that while the
+        // miscdev is registered, `open_data` is initialised.
+        unsafe { (*reg).open_data.as_ptr() }
     }
 }
 
 // SAFETY: The only method is `register()`, which requires a (pinned) mutable `Registration`, so it
-// is safe to pass `&Registration` to multiple threads because it offers no interior mutability,
-// except maybe through `Registration::context`, but it is itself `Sync`.
+// is safe to pass `&Registration` to multiple threads because it offers no interior mutability.
 unsafe impl<T: Sync> Sync for Registration<T> {}
 
-// SAFETY: All functions work from any thread. So as long as the `Registration::context` is
+// SAFETY: All functions work from any thread. So as long as the `Registration::open_data` is
 // `Send`, so is `Registration<T>`. `T` needs to be `Sync` because it's a requirement of
 // `Registration<T>`.
 unsafe impl<T: Send + Sync> Send for Registration<T> {}
@@ -105,7 +131,13 @@ impl<T: Sync> Drop for Registration<T> {
     /// Removes the registration from the kernel if it has completed successfully before.
     fn drop(&mut self) {
         if self.registered {
-            unsafe { bindings::misc_deregister(&mut self.mdev) }
+            // SAFETY: `registered` being `true` indicates that a previous call to  `misc_register`
+            // succeeded.
+            unsafe { bindings::misc_deregister(&mut self.mdev) };
+
+            // SAFETY: The type invariant guarantees that `open_data` is initialised when
+            // `registered` is `true`.
+            unsafe { self.open_data.assume_init_drop() };
         }
     }
 }