* [PATCH v2 1/3] rust: add UnsafePinned type
2025-01-31 15:08 [PATCH v2 0/3] rust: miscdevice: Add additional data to MiscDeviceRegistration Christian Schrefl
@ 2025-01-31 15:08 ` Christian Schrefl
2025-03-26 20:26 ` Benno Lossin
2025-01-31 15:08 ` [PATCH v2 2/3] rust: miscdevice: Add additional data to MiscDeviceRegistration Christian Schrefl
2025-01-31 15:08 ` [PATCH v2 3/3] rust: miscdevice: adjust the rust_misc_device sample to use RegistrationData Christian Schrefl
2 siblings, 1 reply; 5+ messages in thread
From: Christian Schrefl @ 2025-01-31 15:08 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Arnd Bergmann, Greg Kroah-Hartman, Lee Jones,
Daniel Almeida, Danilo Krummrich
Cc: rust-for-linux, linux-kernel, Christian Schrefl
`UnsafePinned<T>` is useful for cases where a value might be shared with C
code but not directly used by it. In particular this is added for
additional data in the `MiscDeviceRegistration` which will be shared
between `fops->open` and the containing struct.
Similar to `Opaque` but guarantees that the value is always initialized
and that the inner value is dropped when `UnsafePinned` is dropped.
This was originally proposed for the IRQ abstractions [0] and is also
useful for other where the inner data may be aliased, but is always valid
and automatic `Drop` is desired.
Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
rust/kernel/types.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 2bbaab83b9d65da667a07e85b3c89c7fa881b53c..3c2f6ac62d161f1187b5e7ade86689eec667ff4d 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -253,6 +253,9 @@ fn drop(&mut self) {
///
/// `Opaque<T>` is meant to be used with FFI objects that are never interpreted by Rust code.
///
+/// In cases where the contained data is only used by Rust, is not allowed to be
+/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
+///
/// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
/// It gets rid of all the usual assumptions that Rust has for a value:
///
@@ -573,3 +576,57 @@ pub enum Either<L, R> {
/// [`NotThreadSafe`]: type@NotThreadSafe
#[allow(non_upper_case_globals)]
pub const NotThreadSafe: NotThreadSafe = PhantomData;
+
+/// Stores a value that may be used from multiple mutable pointers.
+///
+/// `UnsafePinned` gets rid of some of the usual assumptions that Rust has for a value:
+/// - The value is allowed to be mutated, when a `&UnsafePinned<T>` exists on the Rust side.
+/// - No uniqueness for mutable references: it is fine to have multiple `&mut UnsafePinned<T>`
+/// point to the same value.
+///
+/// To avoid the ability to use [`core::mem::swap`] this still needs to be used through a
+/// [`core::pin::Pin`] reference.
+///
+/// This is useful for cases where a value might be shared with C code
+/// but not interpreted by it or in cases where it can not always be guaranteed that the
+/// references are unique.
+///
+/// This is similar to [`Opaque<T>`] but is guaranteed to always contain valid data and will
+/// call the [`Drop`] implementation of `T` when dropped.
+#[repr(transparent)]
+pub struct UnsafePinned<T> {
+ value: UnsafeCell<T>,
+ _pin: PhantomPinned,
+}
+
+impl<T> UnsafePinned<T> {
+ /// Creates a new [`UnsafePinned`] value.
+ pub const fn new(value: T) -> Self {
+ Self {
+ value: UnsafeCell::new(value),
+ _pin: PhantomPinned,
+ }
+ }
+
+ /// Create an [`UnsafePinned`] pin-initializer from the given pin-initializer.
+ pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+ // SAFETY:
+ // - In case of an error in `value` the error is returned, otherwise `slot` is fully
+ // initialized, since `self.value` is initialized and `_pin` is a zero sized type.
+ // - The `Pin` invariants of `self.value` are upheld, since no moving occurs.
+ unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }
+ }
+
+ /// Returns a raw pointer to the contained data.
+ pub const fn get(&self) -> *mut T {
+ UnsafeCell::get(&self.value).cast::<T>()
+ }
+
+ /// Gets the value behind `this`.
+ ///
+ /// This function is useful to get access to the value without creating intermediate
+ /// references.
+ pub const fn raw_get(this: *const Self) -> *mut T {
+ UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()
+ }
+}
--
2.48.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] rust: add UnsafePinned type
2025-01-31 15:08 ` [PATCH v2 1/3] rust: add UnsafePinned type Christian Schrefl
@ 2025-03-26 20:26 ` Benno Lossin
0 siblings, 0 replies; 5+ messages in thread
From: Benno Lossin @ 2025-03-26 20:26 UTC (permalink / raw)
To: Christian Schrefl, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Arnd Bergmann, Greg Kroah-Hartman, Lee Jones,
Daniel Almeida, Danilo Krummrich
Cc: rust-for-linux, linux-kernel
On Fri Jan 31, 2025 at 4:08 PM CET, Christian Schrefl wrote:
> @@ -573,3 +576,57 @@ pub enum Either<L, R> {
> /// [`NotThreadSafe`]: type@NotThreadSafe
> #[allow(non_upper_case_globals)]
> pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +/// Stores a value that may be used from multiple mutable pointers.
> +///
> +/// `UnsafePinned` gets rid of some of the usual assumptions that Rust has for a value:
> +/// - The value is allowed to be mutated, when a `&UnsafePinned<T>` exists on the Rust side.
> +/// - No uniqueness for mutable references: it is fine to have multiple `&mut UnsafePinned<T>`
> +/// point to the same value.
We have another patch series [1] in transit that changes the wording on
the `Opaque<T>` type. I think we should mirror the same wording here.
[1]: https://lore.kernel.org/rust-for-linux/20250305053438.1532397-2-dirk.behme@de.bosch.com/
> +///
> +/// To avoid the ability to use [`core::mem::swap`] this still needs to be used through a
IIRC typing out the whole path makes it also appear in the docs. I think
we should just use `mem::swap` and omit `core` of course you need to
add this to make it work:
/// [`mem::swap`]: core::mem::swap
> +/// [`core::pin::Pin`] reference.
Here, I would link to `Pin`, but say "[pinned](core::pin::Pin) pointer."
instead, since eg `Pin<Box<T>>` also is okay to use.
> +///
> +/// This is useful for cases where a value might be shared with C code
> +/// but not interpreted by it or in cases where it can not always be guaranteed that the
> +/// references are unique.
> +///
> +/// This is similar to [`Opaque<T>`] but is guaranteed to always contain valid data and will
> +/// call the [`Drop`] implementation of `T` when dropped.
> +#[repr(transparent)]
> +pub struct UnsafePinned<T> {
> + value: UnsafeCell<T>,
> + _pin: PhantomPinned,
> +}
> +
> +impl<T> UnsafePinned<T> {
> + /// Creates a new [`UnsafePinned`] value.
> + pub const fn new(value: T) -> Self {
> + Self {
> + value: UnsafeCell::new(value),
> + _pin: PhantomPinned,
> + }
> + }
> +
> + /// Create an [`UnsafePinned`] pin-initializer from the given pin-initializer.
> + pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> + // SAFETY:
> + // - In case of an error in `value` the error is returned, otherwise `slot` is fully
> + // initialized, since `self.value` is initialized and `_pin` is a zero sized type.
> + // - The `Pin` invariants of `self.value` are upheld, since no moving occurs.
> + unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }
Ah this is a bit suboptimal, but I guess there currently isn't a better
way to do this. I'll add it to my list of things to improve with
pin-init.
> + }
> +
> + /// Returns a raw pointer to the contained data.
> + pub const fn get(&self) -> *mut T {
> + UnsafeCell::get(&self.value).cast::<T>()
> + }
> +
> + /// Gets the value behind `this`.
> + ///
> + /// This function is useful to get access to the value without creating intermediate
> + /// references.
> + pub const fn raw_get(this: *const Self) -> *mut T {
> + UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()
Why the cast to `MaybeUninit<T>`? I think this can just be:
UnsafeCell::raw_get(&raw const this.value)
---
Cheers,
Benno
> + }
> +}
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/3] rust: miscdevice: Add additional data to MiscDeviceRegistration
2025-01-31 15:08 [PATCH v2 0/3] rust: miscdevice: Add additional data to MiscDeviceRegistration Christian Schrefl
2025-01-31 15:08 ` [PATCH v2 1/3] rust: add UnsafePinned type Christian Schrefl
@ 2025-01-31 15:08 ` Christian Schrefl
2025-01-31 15:08 ` [PATCH v2 3/3] rust: miscdevice: adjust the rust_misc_device sample to use RegistrationData Christian Schrefl
2 siblings, 0 replies; 5+ messages in thread
From: Christian Schrefl @ 2025-01-31 15:08 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Arnd Bergmann, Greg Kroah-Hartman, Lee Jones,
Daniel Almeida, Danilo Krummrich
Cc: rust-for-linux, linux-kernel, Christian Schrefl
When using the Rust miscdevice bindings, you generally embed the
`MiscDeviceRegistration` within another struct:
struct MyDriverData {
data: SomeOtherData,
misc: MiscDeviceRegistration<MyMiscFile>
}
In the `fops->open` callback of the miscdevice, you are given a
reference to the registration, which allows you to access its fields.
For example, as of commit 284ae0be4dca ("rust: miscdevice: Provide
accessor to pull out miscdevice::this_device") you can access the
internal `struct device`. However, there is still no way to access the
`data` field in the above example, because you only have a reference to
the registration.
Using `container_of` is also not possible to do safely. For example, if
the destructor of `MyDriverData` runs, then the destructor of `data`
would run before the miscdevice is deregistered, so using `container_of`
to access `data` from `fops->open` could result in a UAF. A similar
problem can happen on initialization if `misc` is not the last field to
be initialized.
To provide a safe way to access user-defined data stored next to the
`struct miscdevice`, make `MiscDeviceRegistration` into a container that
can store a user-provided piece of data. This way, `fops->open` can
access that data via the registration, since the data is stored inside
the registration.
The container enforces that the additional user data is initialized
before the miscdevice is registered, and that the miscdevice is
deregistered before the user data is destroyed. This ensures that access
to the userdata is safe.
For the same reasons as in commit 88441d5c6d17 ("rust: miscdevice:
access the `struct miscdevice` from fops->open()"), you cannot access
the user data in any other fops callback than open. This is because a
miscdevice can be deregistered while there are still open files.
A situation where this user data might be required is when a platform
driver acquires a resource in `probe` and wants to use this resource in
the `fops` implementation of a `MiscDevice`.
This solution is similar to the approach used by the initial downstream
Rust-for-Linux/Rust branch [0].
Link: https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/miscdev.rs#L108 [0]
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
rust/kernel/miscdevice.rs | 75 +++++++++++++++++++++++++++++-----------
samples/rust/rust_misc_device.rs | 4 ++-
2 files changed, 58 insertions(+), 21 deletions(-)
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index e14433b2ab9d8fa391474b2ad7e3ed55c64b4d91..dea7d8d1a0366cf2243c7a3888ebfb8a90d6295c 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -9,7 +9,7 @@
//! Reference: <https://www.kernel.org/doc/html/latest/driver-api/misc_devices.html>
use crate::{
- bindings,
+ bindings, container_of,
device::Device,
error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
ffi::{c_int, c_long, c_uint, c_ulong},
@@ -17,7 +17,7 @@
prelude::*,
seq_file::SeqFile,
str::CStr,
- types::{ForeignOwnable, Opaque},
+ types::{ForeignOwnable, Opaque, UnsafePinned},
};
use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
@@ -45,32 +45,46 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
/// # Invariants
///
/// `inner` is a registered misc device.
-#[repr(transparent)]
+#[repr(C)]
#[pin_data(PinnedDrop)]
-pub struct MiscDeviceRegistration<T> {
+pub struct MiscDeviceRegistration<T: MiscDevice> {
#[pin]
inner: Opaque<bindings::miscdevice>,
+ #[pin]
+ data: UnsafePinned<T::RegistrationData>,
_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> {}
+// SAFETY:
+// - It is allowed to call `misc_deregister` on a different thread from where you called
+// `misc_register`.
+// - Only implements `Send` if `MiscDevice::RegistrationData` is also `Send`.
+unsafe impl<T: MiscDevice> Send for MiscDeviceRegistration<T> where T::RegistrationData: Send {}
+
+// SAFETY:
+// - All `&self` methods on this type are written to ensure that it is safe to call them in
+// parallel.
+// - `MiscDevice::RegistrationData` is always `Sync`.
+unsafe impl<T: MiscDevice> Sync for MiscDeviceRegistration<T> {}
impl<T: MiscDevice> MiscDeviceRegistration<T> {
/// Register a misc device.
- pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
+ pub fn register(
+ opts: MiscDeviceOptions,
+ data: impl PinInit<T::RegistrationData, Error>,
+ ) -> impl PinInit<Self, Error> {
try_pin_init!(Self {
+ data <- UnsafePinned::try_pin_init(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::<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.
+ // 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.
+ // * `data` is Initialized before `misc_register` so no race with `fops->open()`
+ // is possible.
// INVARIANT: If this returns `Ok(())`, then the `slot` will contain a registered
// misc device.
to_result(unsafe { bindings::misc_register(slot) })
@@ -93,10 +107,18 @@ pub fn device(&self) -> &Device {
// before the underlying `struct miscdevice` is destroyed.
unsafe { Device::as_ref((*self.as_raw()).this_device) }
}
+
+ /// Access the additional data stored in this registration.
+ pub fn data(&self) -> &T::RegistrationData {
+ // SAFETY:
+ // * No mutable reference to the value contained by `self.data` can ever be created.
+ // * The value contained by `self.data` is valid for the entire lifetime of `&self`.
+ unsafe { &*self.data.get() }
+ }
}
#[pinned_drop]
-impl<T> PinnedDrop for MiscDeviceRegistration<T> {
+impl<T: MiscDevice> 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()) };
@@ -109,6 +131,13 @@ pub trait MiscDevice: Sized {
/// What kind of pointer should `Self` be wrapped in.
type Ptr: ForeignOwnable + Send + Sync;
+ /// The additional data carried by the [`MiscDeviceRegistration`] for this [`MiscDevice`].
+ /// If no additional data is required than the unit type `()` should be used.
+ ///
+ /// This data can be accessed in [`MiscDevice::open()`] using
+ /// [`MiscDeviceRegistration::data()`].
+ type RegistrationData: Sync;
+
/// Called when the misc device is opened.
///
/// The returned pointer will be stored as the private data for the file.
@@ -211,17 +240,23 @@ impl<T: MiscDevice> VtableHelper<T> {
// 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::<MiscDeviceRegistration<T>>() };
+ // This is a miscdevice, so `misc_open()` sets the private data to a pointer to the
+ // associated `struct miscdevice` before calling into this method.
+ let misc_ptr = misc_ptr.cast::<bindings::miscdevice>();
+
+ // SAFETY:
+ // * `misc_open()` ensures that the `struct miscdevice` can't be unregistered and freed
+ // during this call to `fops_open`.
+ // * The `misc_ptr` always points to the `inner` field of a `MiscDeviceRegistration<T>`.
+ // * The `MiscDeviceRegistration<T>` is valid until the `struct miscdevice` was unregistered.
+ let registration = unsafe { &*container_of!(misc_ptr, MiscDeviceRegistration<T>, inner) };
// SAFETY:
// * 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 file = unsafe { File::from_raw_file(raw_file) };
- let ptr = match T::open(file, misc) {
+ let ptr = match T::open(file, registration) {
Ok(ptr) => ptr,
Err(err) => return err.to_errno(),
};
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 40ad7266c2252e5c0b4e91e501ef9ada2eda3b16..779fcfd64119bdd5b4f8be740f7e8336c652b4d3 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -136,7 +136,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
};
try_pin_init!(Self {
- _miscdev <- MiscDeviceRegistration::register(options),
+ _miscdev <- MiscDeviceRegistration::register(options, ()),
})
}
}
@@ -156,6 +156,8 @@ struct RustMiscDevice {
impl MiscDevice for RustMiscDevice {
type Ptr = Pin<KBox<Self>>;
+ type RegistrationData = ();
+
fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Self>>> {
let dev = ARef::from(misc.device());
--
2.48.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/3] rust: miscdevice: adjust the rust_misc_device sample to use RegistrationData.
2025-01-31 15:08 [PATCH v2 0/3] rust: miscdevice: Add additional data to MiscDeviceRegistration Christian Schrefl
2025-01-31 15:08 ` [PATCH v2 1/3] rust: add UnsafePinned type Christian Schrefl
2025-01-31 15:08 ` [PATCH v2 2/3] rust: miscdevice: Add additional data to MiscDeviceRegistration Christian Schrefl
@ 2025-01-31 15:08 ` Christian Schrefl
2 siblings, 0 replies; 5+ messages in thread
From: Christian Schrefl @ 2025-01-31 15:08 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Arnd Bergmann, Greg Kroah-Hartman, Lee Jones,
Daniel Almeida, Danilo Krummrich
Cc: rust-for-linux, linux-kernel, Christian Schrefl
Add a second mutex to the RustMiscDevice, which is shared between all
instances of the device using an Arc and the RegistrationData of
MiscDeviceRegistration.
This is mostly to demonstrate the capability to share data in this way.
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
samples/rust/rust_misc_device.rs | 117 ++++++++++++++++++++++++++++++++++++---
1 file changed, 108 insertions(+), 9 deletions(-)
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 779fcfd64119bdd5b4f8be740f7e8336c652b4d3..050910400c6f05024625136a66ffbeec21d654e8 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -86,6 +86,62 @@
/// return -1;
/// }
///
+/// value++;
+///
+/// // Set shared value to something different
+/// printf("Submitting new shared value (%d)\n", value);
+/// ret = ioctl(fd, RUST_MISC_DEV_SET_SHARED_VALUE, &value);
+/// if (ret < 0) {
+/// perror("ioctl: Failed to submit new value");
+/// close(fd);
+/// return errno;
+/// }
+///
+/// // Close the device file
+/// printf("Closing /dev/rust-misc-device\n");
+/// close(fd);
+///
+/// // Open the device file again
+/// printf("Opening /dev/rust-misc-device again for reading\n");
+/// fd = open("/dev/rust-misc-device", O_RDWR);
+/// if (fd < 0) {
+/// perror("open");
+/// return errno;
+/// }
+///
+/// // Ensure new value was applied
+/// printf("Fetching new value\n");
+/// ret = ioctl(fd, RUST_MISC_DEV_GET_SHARED_VALUE, &new_value);
+/// if (ret < 0) {
+/// perror("ioctl: Failed to fetch the new value");
+/// close(fd);
+/// return errno;
+/// }
+///
+/// if (value != new_value) {
+/// printf("Failed: Committed and retrieved values are different (%d - %d)\n",
+/// value, new_value);
+/// close(fd);
+/// return -1;
+/// }
+///
+/// value = 0;
+/// // Ensure non-shared value is still 0
+/// printf("Fetching new value\n");
+/// ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &new_value);
+/// if (ret < 0) {
+/// perror("ioctl: Failed to fetch the new value");
+/// close(fd);
+/// return errno;
+/// }
+///
+/// if (value != new_value) {
+/// printf("Failed: Committed and retrieved values are different (%d - %d)\n",
+/// value, new_value);
+/// close(fd);
+/// return -1;
+/// }
+///
/// // Close the device file
/// printf("Closing /dev/rust-misc-device\n");
/// close(fd);
@@ -104,7 +160,7 @@
miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
new_mutex,
prelude::*,
- sync::Mutex,
+ sync::{Arc, Mutex},
types::ARef,
uaccess::{UserSlice, UserSliceReader, UserSliceWriter},
};
@@ -112,6 +168,8 @@
const RUST_MISC_DEV_HELLO: u32 = _IO('|' as u32, 0x80);
const RUST_MISC_DEV_GET_VALUE: u32 = _IOR::<i32>('|' as u32, 0x81);
const RUST_MISC_DEV_SET_VALUE: u32 = _IOW::<i32>('|' as u32, 0x82);
+const RUST_MISC_DEV_GET_SHARED_VALUE: u32 = _IOR::<i32>('|' as u32, 0x83);
+const RUST_MISC_DEV_SET_SHARED_VALUE: u32 = _IOW::<i32>('|' as u32, 0x84);
module! {
type: RustMiscDeviceModule,
@@ -129,14 +187,17 @@ struct RustMiscDeviceModule {
impl kernel::InPlaceModule for RustMiscDeviceModule {
fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
- pr_info!("Initialising Rust Misc Device Sample\n");
+ pr_info!("Initializing Rust Misc Device Sample\n");
let options = MiscDeviceOptions {
name: c_str!("rust-misc-device"),
};
try_pin_init!(Self {
- _miscdev <- MiscDeviceRegistration::register(options, ()),
+ _miscdev <- MiscDeviceRegistration::register(
+ options,
+ Arc::pin_init(new_mutex!(Inner { value: 0_i32 }), GFP_KERNEL)?
+ ),
})
}
}
@@ -147,8 +208,9 @@ struct Inner {
#[pin_data(PinnedDrop)]
struct RustMiscDevice {
+ shared: Arc<Mutex<Inner>>,
#[pin]
- inner: Mutex<Inner>,
+ unique: Mutex<Inner>,
dev: ARef<Device>,
}
@@ -156,7 +218,7 @@ struct RustMiscDevice {
impl MiscDevice for RustMiscDevice {
type Ptr = Pin<KBox<Self>>;
- type RegistrationData = ();
+ type RegistrationData = Arc<Mutex<Inner>>;
fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Self>>> {
let dev = ARef::from(misc.device());
@@ -166,7 +228,8 @@ fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Se
KBox::try_pin_init(
try_pin_init! {
RustMiscDevice {
- inner <- new_mutex!( Inner{ value: 0_i32 } ),
+ shared: misc.data().clone(),
+ unique <- new_mutex!(Inner { value: 0_i32 }),
dev: dev,
}
},
@@ -182,6 +245,12 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result
match cmd {
RUST_MISC_DEV_GET_VALUE => me.get_value(UserSlice::new(arg, size).writer())?,
RUST_MISC_DEV_SET_VALUE => me.set_value(UserSlice::new(arg, size).reader())?,
+ RUST_MISC_DEV_GET_SHARED_VALUE => {
+ me.get_shared_value(UserSlice::new(arg, size).writer())?
+ }
+ RUST_MISC_DEV_SET_SHARED_VALUE => {
+ me.set_shared_value(UserSlice::new(arg, size).reader())?
+ }
RUST_MISC_DEV_HELLO => me.hello()?,
_ => {
dev_err!(me.dev, "-> IOCTL not recognised: {}\n", cmd);
@@ -192,7 +261,6 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result
Ok(0)
}
}
-
#[pinned_drop]
impl PinnedDrop for RustMiscDevice {
fn drop(self: Pin<&mut Self>) {
@@ -203,7 +271,7 @@ fn drop(self: Pin<&mut Self>) {
impl RustMiscDevice {
fn set_value(&self, mut reader: UserSliceReader) -> Result<isize> {
let new_value = reader.read::<i32>()?;
- let mut guard = self.inner.lock();
+ let mut guard = self.unique.lock();
dev_info!(
self.dev,
@@ -216,7 +284,38 @@ fn set_value(&self, mut reader: UserSliceReader) -> Result<isize> {
}
fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
- let guard = self.inner.lock();
+ let guard = self.unique.lock();
+ let value = guard.value;
+
+ // Free-up the lock and use our locally cached instance from here
+ drop(guard);
+
+ dev_info!(
+ self.dev,
+ "-> Copying data to userspace (value: {})\n",
+ &value
+ );
+
+ writer.write::<i32>(&value)?;
+ Ok(0)
+ }
+
+ fn set_shared_value(&self, mut reader: UserSliceReader) -> Result<isize> {
+ let new_value = reader.read::<i32>()?;
+ let mut guard = self.shared.lock();
+
+ dev_info!(
+ self.dev,
+ "-> Copying data from userspace (value: {})\n",
+ new_value
+ );
+
+ guard.value = new_value;
+ Ok(0)
+ }
+
+ fn get_shared_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
+ let guard = self.shared.lock();
let value = guard.value;
// Free-up the lock and use our locally cached instance from here
--
2.48.1
^ permalink raw reply related [flat|nested] 5+ messages in thread