diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 301ce5647274ab..a3420273e03e76 100644 --- a/rust/kernel/lib.rs +++ b/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, diff --git a/rust/kernel/miscdev.rs b/rust/kernel/miscdev.rs index b93e423bfb1e7e..f38ec203969d06 100644 --- a/rust/kernel/miscdev.rs +++ b/rust/kernel/miscdev.rs @@ -12,9 +12,13 @@ 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 { registered: bool, mdev: bindings::miscdevice, @@ -22,19 +26,20 @@ pub struct Registration { /// Context initialised on construction and made available to all file instances on /// [`FileOpener::open`]. - pub context: T, + open_data: MaybeUninit, } impl Registration { /// 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 Registration { pub fn new_pinned>( name: &'static CStr, minor: Option, - context: T, + open_data: T, ) -> Result>> { - let mut r = Pin::from(Box::try_new(Self::new(context))?); - r.as_mut().register::(name, minor)?; + let mut r = Pin::from(Box::try_new(Self::new())?); + r.as_mut().register::(name, minor, open_data)?; Ok(r) } @@ -59,6 +64,7 @@ impl Registration { self: Pin<&mut Self>, name: &'static CStr, minor: Option, + 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 Registration { 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 Default for Registration { + fn default() -> Self { + Self::new() + } +} + impl FileOpenAdapter for Registration { 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 Sync for Registration {} -// 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` needs to be `Sync` because it's a requirement of // `Registration`. unsafe impl Send for Registration {} @@ -105,7 +131,13 @@ impl Drop for Registration { /// 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() }; } } }