From 0d8a7c7bf47aa1002e0df792cb1d0652bc824cba Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 10 Dec 2024 09:39:00 +0000 Subject: rust: miscdevice: access file in fops This allows fops to access information about the underlying struct file for the miscdevice. For example, the Binder driver needs to inspect the O_NONBLOCK flag inside the fops->ioctl() hook. Signed-off-by: Alice Ryhl Reviewed-by: Lee Jones Tested-by: Lee Jones Reviewed-by: Danilo Krummrich Link: https://lore.kernel.org/r/20241210-miscdevice-file-param-v3-1-b2a79b666dc5@google.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/miscdevice.rs | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 7e2a79b3ae26..0cb79676c139 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -11,6 +11,7 @@ use crate::{ bindings, error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR}, + fs::File, prelude::*, str::CStr, types::{ForeignOwnable, Opaque}, @@ -103,10 +104,10 @@ pub trait MiscDevice { /// Called when the misc device is opened. /// /// The returned pointer will be stored as the private data for the file. - fn open() -> Result; + fn open(_file: &File) -> Result; /// Called when the misc device is released. - fn release(device: Self::Ptr) { + fn release(device: Self::Ptr, _file: &File) { drop(device); } @@ -117,6 +118,7 @@ pub trait MiscDevice { /// [`kernel::ioctl`]: mod@crate::ioctl fn ioctl( _device: ::Borrowed<'_>, + _file: &File, _cmd: u32, _arg: usize, ) -> Result { @@ -133,6 +135,7 @@ pub trait MiscDevice { #[cfg(CONFIG_COMPAT)] fn compat_ioctl( _device: ::Borrowed<'_>, + _file: &File, _cmd: u32, _arg: usize, ) -> Result { @@ -187,7 +190,10 @@ unsafe extern "C" fn fops_open( return ret; } - let ptr = match T::open() { + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + let ptr = match T::open(unsafe { File::from_raw_file(file) }) { Ok(ptr) => ptr, Err(err) => return err.to_errno(), }; @@ -211,7 +217,10 @@ unsafe extern "C" fn fops_release( // SAFETY: The release call of a file owns the private data. let ptr = unsafe { ::from_foreign(private) }; - T::release(ptr); + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + T::release(ptr, unsafe { File::from_raw_file(file) }); 0 } @@ -229,7 +238,12 @@ unsafe extern "C" fn fops_ioctl( // SAFETY: Ioctl calls can borrow the private data of the file. let device = unsafe { ::borrow(private) }; - match T::ioctl(device, cmd, arg as usize) { + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + let file = unsafe { File::from_raw_file(file) }; + + match T::ioctl(device, file, cmd, arg as usize) { Ok(ret) => ret as c_long, Err(err) => err.to_errno() as c_long, } @@ -249,7 +263,12 @@ unsafe extern "C" fn fops_compat_ioctl( // SAFETY: Ioctl calls can borrow the private data of the file. let device = unsafe { ::borrow(private) }; - match T::compat_ioctl(device, cmd, arg as usize) { + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + let file = unsafe { File::from_raw_file(file) }; + + match T::compat_ioctl(device, file, cmd, arg as usize) { Ok(ret) => ret as c_long, Err(err) => err.to_errno() as c_long, } -- cgit From 88441d5c6d17211bcbd5b429205b09c25598f756 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 10 Dec 2024 09:39:01 +0000 Subject: rust: miscdevice: access the `struct miscdevice` from fops->open() Providing access to the underlying `struct miscdevice` is useful for various reasons. For example, this allows you access the miscdevice's internal `struct device` for use with the `dev_*` printing macros. Note that since the underlying `struct miscdevice` could get freed at any point after the fops->open() call (if misc_deregister is called), only the open call is given access to it. To use `dev_*` printing macros from other fops hooks, take a refcount on `miscdevice->this_device` to keep it alive. See the linked thread for further discussion on the lifetime of `struct miscdevice`. Link: https://lore.kernel.org/r/2024120951-botanist-exhale-4845@gregkh Signed-off-by: Alice Ryhl Reviewed-by: Lee Jones Tested-by: Lee Jones Reviewed-by: Danilo Krummrich Link: https://lore.kernel.org/r/20241210-miscdevice-file-param-v3-2-b2a79b666dc5@google.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/miscdevice.rs | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 0cb79676c139..75a9d26c8001 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -97,14 +97,14 @@ impl PinnedDrop for MiscDeviceRegistration { /// Trait implemented by the private data of an open misc device. #[vtable] -pub trait MiscDevice { +pub trait MiscDevice: Sized { /// 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(_file: &File) -> Result; + fn open(_file: &File, _misc: &MiscDeviceRegistration) -> Result; /// Called when the misc device is released. fn release(device: Self::Ptr, _file: &File) { @@ -182,24 +182,38 @@ const fn create_vtable() -> &'static bindings::file_operations { /// The file must be associated with a `MiscDeviceRegistration`. unsafe extern "C" fn fops_open( inode: *mut bindings::inode, - file: *mut bindings::file, + raw_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) }; + let ret = unsafe { bindings::generic_file_open(inode, raw_file) }; if ret != 0 { return ret; } + // SAFETY: The open call of a file can access the private data. + let misc_ptr = unsafe { (*raw_file).private_data }; + + // 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::>() }; + // SAFETY: - // * The file is valid for the duration of this call. + // * This underlying file is valid for (much longer than) the duration of `T::open`. // * There is no active fdget_pos region on the file on this thread. - let ptr = match T::open(unsafe { File::from_raw_file(file) }) { + let file = unsafe { File::from_raw_file(raw_file) }; + + let ptr = match T::open(file, misc) { 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() }; + // This overwrites the private data with the value specified by the user, changing the type of + // this file's private data. All future accesses to the private data is performed by other + // fops_* methods in this file, which all correctly cast the private data to the new type. + // + // SAFETY: The open call of a file can access the private data. + unsafe { (*raw_file).private_data = ptr.into_foreign().cast_mut() }; 0 } -- cgit From 284ae0be4dcafff0a154c1e69e7430c144a7ddc2 Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Tue, 10 Dec 2024 09:39:02 +0000 Subject: rust: miscdevice: Provide accessor to pull out miscdevice::this_device There are situations where a pointer to a `struct device` will become necessary (e.g. for calling into dev_*() functions). This accessor allows callers to pull this out from the `struct miscdevice`. Signed-off-by: Lee Jones Signed-off-by: Alice Ryhl Tested-by: Lee Jones Reviewed-by: Danilo Krummrich Link: https://lore.kernel.org/r/20241210-miscdevice-file-param-v3-3-b2a79b666dc5@google.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/miscdevice.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 75a9d26c8001..20895e809607 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -10,6 +10,7 @@ use crate::{ bindings, + device::Device, error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR}, fs::File, prelude::*, @@ -85,6 +86,16 @@ impl MiscDeviceRegistration { pub fn as_raw(&self) -> *mut bindings::miscdevice { self.inner.get() } + + /// Access the `this_device` field. + pub fn device(&self) -> &Device { + // SAFETY: This can only be called after a successful register(), which always + // initialises `this_device` with a valid device. Furthermore, the signature of this + // function tells the borrow-checker that the `&Device` reference must not outlive the + // `&MiscDeviceRegistration` used to obtain it, so the last use of the reference must be + // before the underlying `struct miscdevice` is destroyed. + unsafe { Device::as_ref((*self.as_raw()).this_device) } + } } #[pinned_drop] -- cgit From 5bcc8bfe841b29f7d62f4bb7738bb085ecc51aad Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Tue, 3 Dec 2024 12:34:38 +0000 Subject: rust: miscdevice: add fops->show_fdinfo() hook File descriptors should generally provide a fops->show_fdinfo() hook for debugging purposes. Thus, add such a hook to the miscdevice abstractions. Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20241203-miscdevice-showfdinfo-v1-1-7e990732d430@google.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/miscdevice.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 20895e809607..ebc82e7dfc80 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -14,6 +14,7 @@ use crate::{ error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR}, fs::File, prelude::*, + seq_file::SeqFile, str::CStr, types::{ForeignOwnable, Opaque}, }; @@ -152,6 +153,15 @@ pub trait MiscDevice: Sized { ) -> Result { kernel::build_error(VTABLE_DEFAULT_ERROR) } + + /// Show info for this fd. + fn show_fdinfo( + _device: ::Borrowed<'_>, + _m: &SeqFile, + _file: &File, + ) { + kernel::build_error(VTABLE_DEFAULT_ERROR) + } } const fn create_vtable() -> &'static bindings::file_operations { @@ -179,6 +189,7 @@ const fn create_vtable() -> &'static bindings::file_operations { } else { None }, + show_fdinfo: maybe_fn(T::HAS_SHOW_FDINFO, fops_show_fdinfo::), // SAFETY: All zeros is a valid value for `bindings::file_operations`. ..unsafe { MaybeUninit::zeroed().assume_init() } }; @@ -298,3 +309,26 @@ unsafe extern "C" fn fops_compat_ioctl( Err(err) => err.to_errno() as c_long, } } + +/// # Safety +/// +/// - `file` must be a valid file that is associated with a `MiscDeviceRegistration`. +/// - `seq_file` must be a valid `struct seq_file` that we can write to. +unsafe extern "C" fn fops_show_fdinfo( + seq_file: *mut bindings::seq_file, + file: *mut bindings::file, +) { + // 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) }; + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + let file = unsafe { File::from_raw_file(file) }; + // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which + // this method is called. + let m = unsafe { SeqFile::from_raw(seq_file) }; + + T::show_fdinfo(device, m, file); +} -- cgit From 27c7518e7f1ccaaa43eb5f25dc362779d2dc2ccb Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sun, 15 Dec 2024 22:43:53 +0100 Subject: rust: finish using custom FFI integer types In the last kernel cycle we migrated most of the `core::ffi` cases in commit d072acda4862 ("rust: use custom FFI integer types"): Currently FFI integer types are defined in libcore. This commit creates the `ffi` crate and asks bindgen to use that crate for FFI integer types instead of `core::ffi`. This commit is preparatory and no type changes are made in this commit yet. Finish now the few remaining/new cases so that we perform the actual remapping in the next commit as planned. Acked-by: Jocelyn Falempe # drm Link: https://lore.kernel.org/rust-for-linux/CANiq72m_rg42SvZK=bF2f0yEoBLVA33UBhiAsv8THhVu=G2dPA@mail.gmail.com/ Link: https://lore.kernel.org/all/cc9253fa-9d5f-460b-9841-94948fb6580c@redhat.com/ Signed-off-by: Miguel Ojeda --- rust/kernel/miscdevice.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 7e2a79b3ae26..fbd761380384 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -11,16 +11,12 @@ use crate::{ bindings, error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR}, + ffi::{c_int, c_long, c_uint, c_ulong}, prelude::*, str::CStr, types::{ForeignOwnable, Opaque}, }; -use core::{ - ffi::{c_int, c_long, c_uint, c_ulong}, - marker::PhantomData, - mem::MaybeUninit, - pin::Pin, -}; +use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin}; /// Options for creating a misc device. #[derive(Copy, Clone)] -- cgit From 1bae8729e50a900f41e9a1c17ae81113e4cf62b8 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Fri, 13 Sep 2024 22:29:24 +0100 Subject: rust: map `long` to `isize` and `char` to `u8` The following FFI types are replaced compared to `core::ffi`: 1. `char` type is now always mapped to `u8`, since kernel uses `-funsigned-char` on the C code. `core::ffi` maps it to platform default ABI, which can be either signed or unsigned. 2. `long` is now always mapped to `isize`. It's very common in the kernel to use `long` to represent a pointer-sized integer, and in fact `intptr_t` is a typedef of `long` in the kernel. Enforce this mapping rather than mapping to `i32/i64` depending on platform can save us a lot of unnecessary casts. Signed-off-by: Gary Guo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240913213041.395655-5-gary@garyguo.net [ Moved `uaccess` changes from the next commit, since they were irrefutable patterns that Rust >= 1.82.0 warns about. Reworded slightly and reformatted a few documentation comments. Rebased on top of `rust-next`. Added the removal of two casts to avoid Clippy warnings. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/miscdevice.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index fbd761380384..8f88891fb1d2 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -225,7 +225,7 @@ unsafe extern "C" fn fops_ioctl( // SAFETY: Ioctl calls can borrow the private data of the file. let device = unsafe { ::borrow(private) }; - match T::ioctl(device, cmd, arg as usize) { + match T::ioctl(device, cmd, arg) { Ok(ret) => ret as c_long, Err(err) => err.to_errno() as c_long, } @@ -245,7 +245,7 @@ unsafe extern "C" fn fops_compat_ioctl( // SAFETY: Ioctl calls can borrow the private data of the file. let device = unsafe { ::borrow(private) }; - match T::compat_ioctl(device, cmd, arg as usize) { + match T::compat_ioctl(device, cmd, arg) { Ok(ret) => ret as c_long, Err(err) => err.to_errno() as c_long, } -- cgit From 15f2f9313a394f97fb9443271721e9ff1c8d4be4 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sat, 23 Nov 2024 23:28:47 +0100 Subject: rust: use the `build_error!` macro, not the hidden function Code and some examples were using the function, rather than the macro. The macro is what is documented. Thus move users to the macro. Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20241123222849.350287-1-ojeda@kernel.org [ Applied the change to the new miscdevice cases. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/miscdevice.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 8f88891fb1d2..a26b3d31a97b 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -116,7 +116,7 @@ pub trait MiscDevice { _cmd: u32, _arg: usize, ) -> Result { - kernel::build_error(VTABLE_DEFAULT_ERROR) + kernel::build_error!(VTABLE_DEFAULT_ERROR) } /// Handler for ioctls. @@ -132,7 +132,7 @@ pub trait MiscDevice { _cmd: u32, _arg: usize, ) -> Result { - kernel::build_error(VTABLE_DEFAULT_ERROR) + kernel::build_error!(VTABLE_DEFAULT_ERROR) } } -- cgit From 4401565fe92be6ee54a68ea58d80a4076007d5eb Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sat, 23 Nov 2024 23:28:49 +0100 Subject: rust: add `build_error!` to the prelude The sibling `build_assert!` is already in the prelude, it makes sense that a "core"/"language" facility like this is part of the prelude and users should not be defining their own one (thus there should be no risk of future name collisions and we would want to be aware of them anyway). Thus add `build_error!` into the prelude. Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20241123222849.350287-3-ojeda@kernel.org [ Applied the change to the new miscdevice cases. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/miscdevice.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index a26b3d31a97b..9e1b9c0fae9d 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -116,7 +116,7 @@ pub trait MiscDevice { _cmd: u32, _arg: usize, ) -> Result { - kernel::build_error!(VTABLE_DEFAULT_ERROR) + build_error!(VTABLE_DEFAULT_ERROR) } /// Handler for ioctls. @@ -132,7 +132,7 @@ pub trait MiscDevice { _cmd: u32, _arg: usize, ) -> Result { - kernel::build_error!(VTABLE_DEFAULT_ERROR) + build_error!(VTABLE_DEFAULT_ERROR) } } -- cgit From bf2aa7df2687a24ebb52cec4a24443121ac3126d Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 10 Jan 2025 10:14:59 +0000 Subject: miscdevice: rust: use build_error! macro instead of function The function called build_error is an implementation detail of the macro of the same name. Thus, update miscdevice to use the macro rather than the function. See [1] for more information on this. These use the macro with the kernel:: prefix as it has not yet been added to the prelude. Reported-by: Stephen Rothwell Link: https://lore.kernel.org/r/20250110162828.38614c1b@canb.auug.org.au Link: https://lore.kernel.org/all/20241123222849.350287-2-ojeda@kernel.org/ [1] Signed-off-by: Alice Ryhl Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250110101459.536726-1-aliceryhl@google.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/miscdevice.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index ebc82e7dfc80..dfb363630c70 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -134,7 +134,7 @@ pub trait MiscDevice: Sized { _cmd: u32, _arg: usize, ) -> Result { - kernel::build_error(VTABLE_DEFAULT_ERROR) + kernel::build_error!(VTABLE_DEFAULT_ERROR) } /// Handler for ioctls. @@ -151,7 +151,7 @@ pub trait MiscDevice: Sized { _cmd: u32, _arg: usize, ) -> Result { - kernel::build_error(VTABLE_DEFAULT_ERROR) + kernel::build_error!(VTABLE_DEFAULT_ERROR) } /// Show info for this fd. @@ -160,7 +160,7 @@ pub trait MiscDevice: Sized { _m: &SeqFile, _file: &File, ) { - kernel::build_error(VTABLE_DEFAULT_ERROR) + kernel::build_error!(VTABLE_DEFAULT_ERROR) } } -- cgit From 14686571a914833e38eef0f907202f58df4ffcd2 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 20 Nov 2024 06:46:03 -0500 Subject: rust: kernel: change `ForeignOwnable` pointer to mut It is slightly more convenient to operate on mut pointers, and this also properly conveys the desired ownership semantics of the trait. Reviewed-by: Alice Ryhl Reviewed-by: Andreas Hindborg Signed-off-by: Tamir Duberstein Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20241120-borrow-mut-v6-4-80dbadd00951@gmail.com [ Reworded title slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/miscdevice.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel/miscdevice.rs') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index 9e1b9c0fae9d..b3a6cc50b240 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -189,7 +189,7 @@ unsafe extern "C" fn fops_open( }; // SAFETY: The open call of a file owns the private data. - unsafe { (*file).private_data = ptr.into_foreign().cast_mut() }; + unsafe { (*file).private_data = ptr.into_foreign() }; 0 } -- cgit