diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 8d4b719bd83f..ecc142a1b7ea 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -10,6 +10,7 @@ use crate::{ bindings, + container_of, device::Device, error::{ to_result, @@ -32,7 +33,10 @@ Opaque, // }, // }; -use core::marker::PhantomData; +use core::{ + marker::PhantomData, + ops::Deref, // +}; /// Options for creating a misc device. #[derive(Copy, Clone)] @@ -63,37 +67,56 @@ pub const fn into_raw(self) -> bindings::miscdevice { /// - Deregistration occurs exactly once in [`Drop`] via `misc_deregister()`. /// - `inner` wraps a valid, pinned `miscdevice` created using /// [`MiscDeviceOptions::into_raw`]. -#[repr(transparent)] +#[repr(C)] #[pin_data(PinnedDrop)] pub struct MiscDeviceRegistration { #[pin] inner: Opaque, - _t: PhantomData, + #[pin] + data: T, } // SAFETY: It is allowed to call `misc_deregister` on a different thread from where you called -// `misc_register`. -unsafe impl Send for MiscDeviceRegistration {} +// `misc_register`, and `T` is `Send`. +unsafe impl Send for MiscDeviceRegistration {} // SAFETY: All `&self` methods on this type are written to ensure that it is safe to call them in -// parallel. -unsafe impl Sync for MiscDeviceRegistration {} +// parallel, and `T` is `Sync`. +unsafe impl Sync for MiscDeviceRegistration {} impl MiscDeviceRegistration { /// Register a misc device. - pub fn register(opts: MiscDeviceOptions) -> impl PinInit { + /// + /// # Safety + /// + /// The caller must not `mem::forget()` the returned [`MiscDeviceRegistration`] or otherwise + /// prevent its [`Drop`] implementation from running. + pub unsafe fn register<'a>( + opts: MiscDeviceOptions, + data: impl PinInit + 'a, + ) -> impl PinInit + 'a + where + T: 'a, + { try_pin_init!(Self { + data <- data, inner <- Opaque::try_ffi_init(move |slot: *mut bindings::miscdevice| { // SAFETY: The initializer can write to the provided `slot`. unsafe { slot.write(opts.into_raw::()) }; // SAFETY: We just wrote the misc device options to the slot. The miscdevice will // get unregistered before `slot` is deallocated because the memory is pinned and - // the destructor of this type deallocates the memory. + // the caller guarantees that the destructor of this type will run. + // + // The `data` field is already initialized and is only dropped after + // `misc_deregister` in `PinnedDrop::drop`. Since `T: Sync` and the caller + // guarantees that `Drop` will run before `T` becomes invalid, it is sound for the + // `open` callback to access `&MiscDeviceRegistration` until `misc_deregister` + // returns. + // // INVARIANT: If this returns `Ok(())`, then the `slot` will contain a registered // misc device. to_result(unsafe { bindings::misc_register(slot) }) }), - _t: PhantomData, }) } @@ -113,6 +136,14 @@ pub fn device(&self) -> &Device { } } +impl Deref for MiscDeviceRegistration { + type Target = T; + #[inline] + fn deref(&self) -> &T { + &self.data + } +} + #[pinned_drop] impl PinnedDrop for MiscDeviceRegistration { fn drop(self: Pin<&mut Self>) { @@ -121,19 +152,23 @@ fn drop(self: Pin<&mut Self>) { } } -/// Trait implemented by the private data of an open misc device. +/// Trait implemented by the data stored in a [`MiscDeviceRegistration`]. #[vtable] -pub trait MiscDevice: Sized { - /// What kind of pointer should `Self` be wrapped in. - type Ptr: ForeignOwnable + Send + Sync; +pub trait MiscDevice: Sized + Sync { + /// The type of the data stored in each file for this miscdevice. + /// + /// This type must be pointer-sized, so it will usually be a [`KBox`] or [`Arc`]. + /// + /// [`Arc`]: crate::sync::Arc + type File: ForeignOwnable + Send + Sync + 'static; /// Called when the misc device is opened. /// /// The returned pointer will be stored as the private data for the file. - fn open(_file: &File, _misc: &MiscDeviceRegistration) -> Result; + fn open(_file: &File, _misc: &MiscDeviceRegistration) -> Result; /// Called when the misc device is released. - fn release(device: Self::Ptr, _file: &File) { + fn release(device: Self::File, _file: &File) { drop(device); } @@ -146,7 +181,7 @@ fn release(device: Self::Ptr, _file: &File) { /// error, the kernel will complete initialization of the VMA according to the properties of /// `vma`. fn mmap( - _device: ::Borrowed<'_>, + _device: ::Borrowed<'_>, _file: &File, _vma: &VmaNew, ) -> Result { @@ -154,12 +189,12 @@ fn mmap( } /// Read from this miscdevice. - fn read_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterDest<'_>) -> Result { + fn read_iter(_kiocb: Kiocb<'_, Self::File>, _iov: &mut IovIterDest<'_>) -> Result { build_error!(VTABLE_DEFAULT_ERROR) } /// Write to this miscdevice. - fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Result { + fn write_iter(_kiocb: Kiocb<'_, Self::File>, _iov: &mut IovIterSource<'_>) -> Result { build_error!(VTABLE_DEFAULT_ERROR) } @@ -169,7 +204,7 @@ fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Res /// /// [`kernel::ioctl`]: mod@crate::ioctl fn ioctl( - _device: ::Borrowed<'_>, + _device: ::Borrowed<'_>, _file: &File, _cmd: u32, _arg: usize, @@ -186,7 +221,7 @@ fn ioctl( /// provided, then `compat_ptr_ioctl` will be used instead. #[cfg(CONFIG_COMPAT)] fn compat_ioctl( - _device: ::Borrowed<'_>, + _device: ::Borrowed<'_>, _file: &File, _cmd: u32, _arg: usize, @@ -196,7 +231,7 @@ fn compat_ioctl( /// Show info for this fd. fn show_fdinfo( - _device: ::Borrowed<'_>, + _device: ::Borrowed<'_>, _m: &SeqFile, _file: &File, ) { @@ -220,13 +255,14 @@ impl MiscdeviceVTable { } // SAFETY: The open call of a file can access the private data. - let misc_ptr = unsafe { (*raw_file).private_data }; + let misc_ptr = Opaque::cast_from(unsafe { (*raw_file).private_data }.cast()); // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the - // associated `struct miscdevice` before calling into this method. Furthermore, - // `misc_open()` ensures that the miscdevice can't be unregistered and freed during this - // call to `fops_open`. - let misc = unsafe { &*misc_ptr.cast::>() }; + // associated `struct miscdevice` (which is the `inner` field of a valid + // `MiscDeviceRegistration`) before calling into this method. Furthermore, `misc_open()` + // ensures that the miscdevice can't be unregistered and freed during this call to + // `fops_open`, and `T: Sync` allows sharing `&MiscDeviceRegistration` across threads. + let misc = unsafe { &*container_of!(misc_ptr, MiscDeviceRegistration, inner) }; // SAFETY: // * This underlying file is valid for (much longer than) the duration of `T::open`. @@ -257,7 +293,7 @@ impl MiscdeviceVTable { // SAFETY: The release call of a file owns the private data. let private = unsafe { (*file).private_data }; // SAFETY: The release call of a file owns the private data. - let ptr = unsafe { ::from_foreign(private) }; + let ptr = unsafe { ::from_foreign(private) }; // SAFETY: // * The file is valid for the duration of this call. @@ -320,7 +356,7 @@ impl MiscdeviceVTable { // SAFETY: This is a Rust Miscdevice, so we call `into_foreign` in `open` and // `from_foreign` in `release`, and `fops_mmap` is guaranteed to be called between those // two operations. - let device = unsafe { ::borrow(private.cast()) }; + let device = unsafe { ::borrow(private) }; // SAFETY: The caller provides a vma that is undergoing initial VMA setup. let area = unsafe { VmaNew::from_raw(vma) }; // SAFETY: @@ -341,7 +377,7 @@ impl MiscdeviceVTable { // SAFETY: The ioctl call of a file can access the private data. let private = unsafe { (*file).private_data }; // SAFETY: Ioctl calls can borrow the private data of the file. - let device = unsafe { ::borrow(private) }; + let device = unsafe { ::borrow(private) }; // SAFETY: // * The file is valid for the duration of this call. @@ -366,7 +402,7 @@ impl MiscdeviceVTable { // SAFETY: The compat ioctl call of a file can access the private data. let private = unsafe { (*file).private_data }; // SAFETY: Ioctl calls can borrow the private data of the file. - let device = unsafe { ::borrow(private) }; + let device = unsafe { ::borrow(private) }; // SAFETY: // * The file is valid for the duration of this call. @@ -387,7 +423,7 @@ impl MiscdeviceVTable { // SAFETY: The release call of a file owns the private data. let private = unsafe { (*file).private_data }; // SAFETY: Ioctl calls can borrow the private data of the file. - let device = unsafe { ::borrow(private) }; + let device = unsafe { ::borrow(private) }; // SAFETY: // * The file is valid for the duration of this call. // * There is no active fdget_pos region on the file on this thread. diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs index 41e26c825060..f5b2b00b933a 100644 --- a/samples/rust/rust_misc_device.rs +++ b/samples/rust/rust_misc_device.rs @@ -156,7 +156,11 @@ fn init(_module: &'static ThisModule) -> impl PinInit { }; try_pin_init!(Self { - _miscdev <- MiscDeviceRegistration::register(options), + // SAFETY: `_miscdev` is stored in `RustMiscDeviceModule` and dropped when the module + // is unloaded; `mem::forget()` is never called on it. + _miscdev <- unsafe { + MiscDeviceRegistration::register(options, try_pin_init!(RustMiscDevice {})) + }, }) } } @@ -166,8 +170,13 @@ struct Inner { buffer: KVVec, } +/// Data shared by all open files for the same instance of this miscdevice. +#[pin_data] +struct RustMiscDevice {} + +/// Data that is stored per file for this miscdevice. #[pin_data(PinnedDrop)] -struct RustMiscDevice { +struct RustMiscDeviceFile { #[pin] inner: Mutex, dev: ARef, @@ -175,16 +184,16 @@ struct RustMiscDevice { #[vtable] impl MiscDevice for RustMiscDevice { - type Ptr = Pin>; + type File = Pin>; - fn open(_file: &File, misc: &MiscDeviceRegistration) -> Result>> { + fn open(_file: &File, misc: &MiscDeviceRegistration) -> Result { let dev = ARef::from(misc.device()); dev_info!(dev, "Opening Rust Misc Device Sample\n"); KBox::try_pin_init( try_pin_init! { - RustMiscDevice { + RustMiscDeviceFile { inner <- new_mutex!(Inner { value: 0_i32, buffer: KVVec::new(), @@ -196,7 +205,7 @@ fn open(_file: &File, misc: &MiscDeviceRegistration) -> Result, iov: &mut IovIterDest<'_>) -> Result { + fn read_iter(mut kiocb: Kiocb<'_, Self::File>, iov: &mut IovIterDest<'_>) -> Result { let me = kiocb.file(); dev_info!(me.dev, "Reading from Rust Misc Device Sample\n"); @@ -207,7 +216,7 @@ fn read_iter(mut kiocb: Kiocb<'_, Self::Ptr>, iov: &mut IovIterDest<'_>) -> Resu Ok(read) } - fn write_iter(mut kiocb: Kiocb<'_, Self::Ptr>, iov: &mut IovIterSource<'_>) -> Result { + fn write_iter(mut kiocb: Kiocb<'_, Self::File>, iov: &mut IovIterSource<'_>) -> Result { let me = kiocb.file(); dev_info!(me.dev, "Writing to Rust Misc Device Sample\n"); @@ -223,7 +232,7 @@ fn write_iter(mut kiocb: Kiocb<'_, Self::Ptr>, iov: &mut IovIterSource<'_>) -> R Ok(len) } - fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result { + fn ioctl(me: Pin<&RustMiscDeviceFile>, _file: &File, cmd: u32, arg: usize) -> Result { dev_info!(me.dev, "IOCTLing Rust Misc Device Sample\n"); // Treat the ioctl argument as a user pointer. @@ -245,13 +254,13 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result } #[pinned_drop] -impl PinnedDrop for RustMiscDevice { +impl PinnedDrop for RustMiscDeviceFile { fn drop(self: Pin<&mut Self>) { dev_info!(self.dev, "Exiting the Rust Misc Device Sample\n"); } } -impl RustMiscDevice { +impl RustMiscDeviceFile { fn set_value(&self, mut reader: UserSliceReader) -> Result { let new_value = reader.read::()?; let mut guard = self.inner.lock();