On Tue, Oct 01, 2024 at 08:22:22AM GMT, Alice Ryhl wrote: > Provide a `MiscDevice` trait that lets you specify the file operations > that you wish to provide for your misc device. For now, only three file > operations are provided: open, close, ioctl. > > These abstractions only support MISC_DYNAMIC_MINOR. This enforces that > new miscdevices should not hard-code a minor number. > > When implementing ioctl, the Result type is used. This means that you > can choose to return either of: > * An integer of type isize. > * An errno using the kernel::error::Error type. > When returning an isize, the integer is returned verbatim. It's mainly > intended for returning positive integers to userspace. However, it is > technically possible to return errors via the isize return value too. > > To avoid having a dependency on files, this patch does not provide the > file operations callbacks a pointer to the file. This means that they > cannot check file properties such as O_NONBLOCK (which Binder needs). > Support for that can be added as a follow-up. > > To avoid having a dependency on vma, this patch does not provide any way > to implement mmap (which Binder needs). Support for that can be added as > a follow-up. > > Rust Binder will use these abstractions to create the /dev/binder file > when binderfs is disabled. > > Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx> > --- > rust/bindings/bindings_helper.h | 1 + > rust/kernel/lib.rs | 1 + > rust/kernel/miscdevice.rs | 241 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 243 insertions(+) > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > index ae82e9c941af..84303bf221dd 100644 > --- a/rust/bindings/bindings_helper.h > +++ b/rust/bindings/bindings_helper.h > @@ -15,6 +15,7 @@ > #include <linux/firmware.h> > #include <linux/jiffies.h> > #include <linux/mdio.h> > +#include <linux/miscdevice.h> > #include <linux/phy.h> > #include <linux/refcount.h> > #include <linux/sched.h> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index 22a3bfa5a9e9..e268eae54c81 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -39,6 +39,7 @@ > #[cfg(CONFIG_KUNIT)] > pub mod kunit; > pub mod list; > +pub mod miscdevice; > #[cfg(CONFIG_NET)] > pub mod net; > pub mod page; > diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs > new file mode 100644 > index 000000000000..cbd5249b5b45 > --- /dev/null > +++ b/rust/kernel/miscdevice.rs > @@ -0,0 +1,241 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +// Copyright (C) 2024 Google LLC. > + > +//! Miscdevice support. > +//! > +//! C headers: [`include/linux/miscdevice.h`](srctree/include/linux/miscdevice.h). > +//! > +//! Reference: <https://www.kernel.org/doc/html/latest/driver-api/misc_devices.html> > + > +use crate::{ > + bindings, > + error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR}, > + prelude::*, > + str::CStr, > + types::{ForeignOwnable, Opaque}, > +}; > +use core::{ > + ffi::{c_int, c_long, c_uint, c_ulong}, > + marker::PhantomData, > + mem::MaybeUninit, > + pin::Pin, > +}; > + > +/// Options for creating a misc device. > +#[derive(Copy, Clone)] > +pub struct MiscDeviceOptions { > + /// The name of the miscdevice. > + pub name: &'static CStr, > +} > + > +impl MiscDeviceOptions { > + /// Create a raw `struct miscdev` ready for registration. > + pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice { > + // SAFETY: All zeros is valid for this C type. > + let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() }; > + result.minor = bindings::MISC_DYNAMIC_MINOR as _; > + result.name = self.name.as_char_ptr(); > + result.fops = create_vtable::<T>(); > + result > + } > +} > + > +/// A registration of a miscdevice. > +/// > +/// # Invariants > +/// > +/// `inner` is a registered misc device. > +#[repr(transparent)] > +#[pin_data(PinnedDrop)] > +pub struct MiscDeviceRegistration<T> { > + #[pin] > + inner: Opaque<bindings::miscdevice>, > + _t: PhantomData<T>, > +} > + > +// SAFETY: It is allowed to call `misc_deregister` on a different thread from where you called > +// `misc_register`. > +unsafe impl<T> Send for MiscDeviceRegistration<T> {} > +// SAFETY: All `&self` methods on this type are written to ensure that it is safe to call them in > +// parallel. > +unsafe impl<T> Sync for MiscDeviceRegistration<T> {} > + > +impl<T: MiscDevice> MiscDeviceRegistration<T> { > + /// Register a misc device. > + pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> { > + try_pin_init!(Self { > + 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::<T>()) }; > + > + // 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. > + // INVARIANT: If this returns `Ok(())`, then the `slot` will contain a registered > + // misc device. > + to_result(unsafe { bindings::misc_register(slot) }) > + }), > + _t: PhantomData, > + }) > + } > + > + /// Returns a raw pointer to the misc device. > + pub fn as_raw(&self) -> *mut bindings::miscdevice { > + self.inner.get() > + } > +} > + > +#[pinned_drop] > +impl<T> PinnedDrop for MiscDeviceRegistration<T> { > + fn drop(self: Pin<&mut Self>) { > + // SAFETY: We know that the device is registered by the type invariants. > + unsafe { bindings::misc_deregister(self.inner.get()) }; > + } > +} > + > +/// Trait implemented by the private data of an open misc device. > +#[vtable] > +pub trait MiscDevice { > + /// What kind of pointer should `Self` be wrapped in. > + type Ptr: ForeignOwnable + Send + Sync; > + > + /// Called when the misc device is opened. > + /// > + /// The returned pointer will be stored as the private data for the file. > + fn open() -> Result<Self::Ptr>; > + > + /// Called when the misc device is released. > + fn release(device: Self::Ptr) { > + drop(device); > + } > + > + /// Handler for ioctls. > + /// > + /// The `cmd` argument is usually manipulated using the utilties in [`kernel::ioctl`]. > + /// > + /// [`kernel::ioctl`]: mod@crate::ioctl > + fn ioctl( > + _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>, > + _cmd: u32, > + _arg: usize, > + ) -> Result<isize> { > + kernel::build_error(VTABLE_DEFAULT_ERROR) > + } > + > + /// Handler for ioctls. > + /// > + /// Used for 32-bit userspace on 64-bit platforms. > + /// > + /// This method is optional and only needs to be provided if the ioctl relies on structures > + /// that have different layout on 32-bit and 64-bit userspace. If no implementation is > + /// provided, then `compat_ptr_ioctl` will be used instead. > + #[cfg(CONFIG_COMPAT)] > + fn compat_ioctl( > + _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>, > + _cmd: u32, > + _arg: usize, > + ) -> Result<isize> { > + kernel::build_error(VTABLE_DEFAULT_ERROR) > + } > +} > + > +const fn create_vtable<T: MiscDevice>() -> &'static bindings::file_operations { > + const fn maybe_fn<T: Copy>(check: bool, func: T) -> Option<T> { > + if check { > + Some(func) > + } else { > + None > + } > + } > + > + struct VtableHelper<T: MiscDevice> { > + _t: PhantomData<T>, > + } > + impl<T: MiscDevice> VtableHelper<T> { > + const VTABLE: bindings::file_operations = bindings::file_operations { > + open: Some(fops_open::<T>), > + release: Some(fops_release::<T>), > + unlocked_ioctl: maybe_fn(T::HAS_IOCTL, fops_ioctl::<T>), > + #[cfg(CONFIG_COMPAT)] > + compat_ioctl: if T::HAS_COMPAT_IOCTL { > + Some(fops_compat_ioctl::<T>) > + } else if T::HAS_IOCTL { > + Some(bindings::compat_ptr_ioctl) > + } else { > + None > + }, > + ..unsafe { MaybeUninit::zeroed().assume_init() } > + }; > + } > + > + &VtableHelper::<T>::VTABLE > +} > + > +unsafe extern "C" fn fops_open<T: MiscDevice>( > + inode: *mut bindings::inode, > + file: *mut bindings::file, > +) -> c_int { > + // SAFETY: The pointers are valid and for a file being opened. > + let ret = unsafe { bindings::generic_file_open(inode, file) }; > + if ret != 0 { > + return ret; > + } Do you have code where that function is used? Because this looks wrong or at least I don't understand from just a glance whether that generic_file_open() call makes sense. Illustrating how we get from opening /dev/binder to this call would help. > + > + let ptr = match T::open() { > + Ok(ptr) => ptr, > + Err(err) => return err.to_errno(), > + }; > + > + // SAFETY: The open call of a file owns the private data. > + unsafe { (*file).private_data = ptr.into_foreign().cast_mut() }; > + > + 0 > +} > + > +unsafe extern "C" fn fops_release<T: MiscDevice>( > + _inode: *mut bindings::inode, > + file: *mut bindings::file, > +) -> c_int { > + // 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 { <T::Ptr as ForeignOwnable>::from_foreign(private) }; > + > + T::release(ptr); > + > + 0 > +} > + > +unsafe extern "C" fn fops_ioctl<T: MiscDevice>( > + file: *mut bindings::file, > + cmd: c_uint, > + arg: c_ulong, > +) -> c_long { > + // 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 { <T::Ptr as ForeignOwnable>::borrow(private) }; > + > + match T::ioctl(device, cmd as u32, arg as usize) { > + Ok(ret) => ret as c_long, > + Err(err) => err.to_errno() as c_long, > + } > +} > + > +#[cfg(CONFIG_COMPAT)] > +unsafe extern "C" fn fops_compat_ioctl<T: MiscDevice>( > + file: *mut bindings::file, > + cmd: c_uint, > + arg: c_ulong, > +) -> c_long { > + // 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 { <T::Ptr as ForeignOwnable>::borrow(private) }; > + > + match T::compat_ioctl(device, cmd as u32, arg as usize) { > + Ok(ret) => ret as c_long, > + Err(err) => err.to_errno() as c_long, > + } > +} > > -- > 2.46.1.824.gd892dcdcdd-goog >
