rust: device: introduce Device::drvdata()

In C dev_get_drvdata() has specific requirements under which it is valid to access the returned pointer. That is, drivers have to ensure that (1) for the duration the returned pointer is accessed the driver is bound and remains to be bound to the corresponding device, (2) the returned void * is treated according to the driver's private data type, i.e. according to what has been passed to dev_set_drvdata(). In Rust, (1) can be ensured by simply requiring the Bound device context, i.e. provide the drvdata() method for Device<Bound> only. For (2) we would usually make the device type generic over the driver type, e.g. Device<T: Driver>, where <T as Driver>::Data is the type of the driver's private data. However, a device does not have a driver type known at compile time and may be bound to multiple drivers throughout its lifetime. Hence, in order to be able to provide a safe accessor for the driver's device private data, we have to do the type check on runtime. This is achieved by letting a driver assert the expected type, which is then compared to a type hash stored in struct device_private when dev_set_drvdata() is called. Example: // `dev` is a `&Device<Bound>`. let data = dev.drvdata::<SampleDriver>()?; There are two aspects to note: (1) Technically, the same check could be achieved by comparing the struct device_driver pointer of struct device with the struct device_driver pointer of the driver struct (e.g. struct pci_driver). However, this would - in addition the pointer comparison - require to tie back the private driver data type to the struct device_driver pointer of the driver struct to prove correctness. Besides that, accessing the driver struct (stored in the module structure) isn't trivial and would result into horrible code and API ergonomics. (2) Having a direct accessor to the driver's private data is not commonly required (at least in Rust): Bus callback methods already provide access to the driver's device private data through a &self argument, while other driver entry points such as IRQs, workqueues, timers, IOCTLs, etc. have their own private data with separate ownership and lifetime. In other words, a driver's device private data is only relevant for driver model contexts (such a file private is only relevant for file contexts). Having that said, the motivation for accessing the driver's device private data with Device<Bound>::drvdata() are interactions between drivers. For instance, when an auxiliary driver calls back into its parent, the parent has to be capable to derive its private data from the corresponding device (i.e. the parent of the auxiliary device). Reviewed-by: Alice Ryhl <aliceryhl@google.com> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> [ * Remove unnecessary `const _: ()` block, * rename type_id_{store,match}() to {set,match}_type_id(), * assert size_of::<bindings::driver_type>() >= size_of::<TypeId>(), * add missing check in case Device::drvdata() is called from probe(). - Danilo ] Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Danilo Krummrich committed 1 year ago

Commit 6f61a261 parent 6bbaa933 files · +103 −3Browse files

Unified · Split

3 files

drivers/base/base.h +16 −0
rust/bindings/bindings_helper.h +6 −0
rust/kernel/device.rs +81 −3

drivers/base/base.h+16−0

 };
 #define to_driver(obj) container_of(obj, struct driver_private, kobj)
+#ifdef CONFIG_RUST
+/**
+ * struct driver_type - Representation of a Rust driver type.
+ */
+struct driver_type {
+	/**
+	 * @id: Representation of core::any::TypeId.
+	 */
+	u8 id[16];
+} __packed;
+#endif
 /**
  * struct device_private - structure to hold the private to the driver core portions of the device structure.
  *
  * @async_driver - pointer to device driver awaiting probe via async_probe
  * @device - pointer back to the struct device that this structure is
  * associated with.
+ * @driver_type - The type of the bound Rust driver.
  * @dead - This device is currently either in the process of or has been
  *	removed from the system. Any asynchronous events scheduled for this
  *	device should exit without taking any action.
 	const struct device_driver *async_driver;
 	char *deferred_probe_reason;
 	struct device *device;
+#ifdef CONFIG_RUST
+	struct driver_type driver_type;
+#endif
 	u8 dead:1;
 };
 #define to_device_private_parent(obj)	\

rust/bindings/bindings_helper.h+6−0

 #include <linux/xarray.h>
 #include <trace/events/rust_sample.h>
+/*
+ * The driver-core Rust code needs to know about some C driver-core private
+ * structures.
+ */
+#include <../../drivers/base/base.h>
 #if defined(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
 // Used by `#[export]` in `drivers/gpu/drm/drm_panic_qr.rs`.
 #include <drm/drm_panic.h>

rust/kernel/device.rs+81−3

@@ -10,13 +10,16 @@ use crate::{ · 9 unmodified lines
10	sync::aref::ARef,	10	sync::aref::ARef,
11	types::{ForeignOwnable, Opaque},	11	types::{ForeignOwnable, Opaque},
12	};	12	};
13	use core::{marker::PhantomData, ptr};	13	use core::{any::TypeId, marker::PhantomData, ptr};
14		14
15	#[cfg(CONFIG_PRINTK)]	15	#[cfg(CONFIG_PRINTK)]
16	use crate::c_str;	16	use crate::c_str;
17		17
18	pub mod property;	18	pub mod property;
19		19
		20	// Assert that we can `read()` / `write()` a `TypeId` instance from / into `struct driver_type`.
		21	static_assert!(core::mem::size_of::<bindings::driver_type>() >= core::mem::size_of::<TypeId>());
		22
20	/// The core representation of a device in the kernel's driver model.	23	/// The core representation of a device in the kernel's driver model.
21	///	24	///
22	/// This structure represents the Rust abstraction for a C `struct device`. A [`Device`] can either	25	/// This structure represents the Rust abstraction for a C `struct device`. A [`Device`] can either
@@ -198,12 +201,29 @@ impl Device { · 175 unmodified lines
198	}	201	}
199		202
200	impl Device<CoreInternal> {	203	impl Device<CoreInternal> {
		204	fn set_type_id<T: 'static>(&self) {
		205	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
		206	let private = unsafe { (*self.as_raw()).p };
		207
		208	// SAFETY: For a bound device (implied by the `CoreInternal` device context), `private` is
		209	// guaranteed to be a valid pointer to a `struct device_private`.
		210	let driver_type = unsafe { &raw mut (*private).driver_type };
		211
		212	// SAFETY: `driver_type` is valid for (unaligned) writes of a `TypeId`.
		213	unsafe {
		214	driver_type
		215	.cast::<TypeId>()
		216	.write_unaligned(TypeId::of::<T>())
		217	};
		218	}
		219
201	/// Store a pointer to the bound driver's private data.	220	/// Store a pointer to the bound driver's private data.
202	pub fn set_drvdata<T: 'static>(&self, data: impl PinInit<T, Error>) -> Result {	221	pub fn set_drvdata<T: 'static>(&self, data: impl PinInit<T, Error>) -> Result {
203	let data = KBox::pin_init(data, GFP_KERNEL)?;	222	let data = KBox::pin_init(data, GFP_KERNEL)?;
204		223
205	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.	224	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
206	unsafe { bindings::dev_set_drvdata(self.as_raw(), data.into_foreign().cast()) };	225	unsafe { bindings::dev_set_drvdata(self.as_raw(), data.into_foreign().cast()) };
		226	self.set_type_id::<T>();
207		227
208	Ok(())	228	Ok(())
209	}	229	}
@@ -219,6 +239,9 @@ impl Device<CoreInternal> { · 9 unmodified lines
219	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.	239	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
220	let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };	240	let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
221		241
		242	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
		243	unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };
		244
222	// SAFETY:	245	// SAFETY:
223	// - By the safety requirements of this function, `ptr` comes from a previous call to	246	// - By the safety requirements of this function, `ptr` comes from a previous call to
224	// `into_foreign()`.	247	// `into_foreign()`.
@@ -235,7 +258,23 @@ impl Device<CoreInternal> { · 10 unmodified lines
235	/// [`Device::drvdata_obtain`].	258	/// [`Device::drvdata_obtain`].
236	/// - The type `T` must match the type of the `ForeignOwnable` previously stored by	259	/// - The type `T` must match the type of the `ForeignOwnable` previously stored by
237	/// [`Device::set_drvdata`].	260	/// [`Device::set_drvdata`].
238	pub unsafe fn drvdata_borrow<T: ForeignOwnable>(&self) -> T::Borrowed<'_> {	261	pub unsafe fn drvdata_borrow<T: 'static>(&self) -> Pin<&T> {
		262	// SAFETY: `drvdata_unchecked()` has the exact same safety requirements as the ones
		263	// required by this method.
		264	unsafe { self.drvdata_unchecked() }
		265	}
		266	}
		267
		268	impl Device<Bound> {
		269	/// Borrow the driver's private data bound to this [`Device`].
		270	///
		271	/// # Safety
		272	///
		273	/// - Must only be called after a preceding call to [`Device::set_drvdata`] and before
		274	/// [`Device::drvdata_obtain`].
		275	/// - The type `T` must match the type of the `ForeignOwnable` previously stored by
		276	/// [`Device::set_drvdata`].
		277	unsafe fn drvdata_unchecked<T: 'static>(&self) -> Pin<&T> {
239	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.	278	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
240	let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };	279	let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
241		280
@@ -244,7 +283,46 @@ impl Device<CoreInternal> { · 2 unmodified lines
244	// `into_foreign()`.	283	// `into_foreign()`.
245	// - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()`	284	// - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()`
246	// in `into_foreign()`.	285	// in `into_foreign()`.
247	unsafe { T::borrow(ptr.cast()) }	286	unsafe { Pin::<KBox<T>>::borrow(ptr.cast()) }
		287	}
		288
		289	fn match_type_id<T: 'static>(&self) -> Result {
		290	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
		291	let private = unsafe { (*self.as_raw()).p };
		292
		293	// SAFETY: For a bound device, `private` is guaranteed to be a valid pointer to a
		294	// `struct device_private`.
		295	let driver_type = unsafe { &raw mut (*private).driver_type };
		296
		297	// SAFETY:
		298	// - `driver_type` is valid for (unaligned) reads of a `TypeId`.
		299	// - A bound device guarantees that `driver_type` contains a valid `TypeId` value.
		300	let type_id = unsafe { driver_type.cast::<TypeId>().read_unaligned() };
		301
		302	if type_id != TypeId::of::<T>() {
		303	return Err(EINVAL);
		304	}
		305
		306	Ok(())
		307	}
		308
		309	/// Access a driver's private data.
		310	///
		311	/// Returns a pinned reference to the driver's private data or [`EINVAL`] if it doesn't match
		312	/// the asserted type `T`.
		313	pub fn drvdata<T: 'static>(&self) -> Result<Pin<&T>> {
		314	// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
		315	if unsafe { bindings::dev_get_drvdata(self.as_raw()) }.is_null() {
		316	return Err(ENOENT);
		317	}
		318
		319	self.match_type_id::<T>()?;
		320
		321	// SAFETY:
		322	// - The above check of `dev_get_drvdata()` guarantees that we are called after
		323	// `set_drvdata()` and before `drvdata_obtain()`.
		324	// - We've just checked that the type of the driver's private data is in fact `T`.
		325	Ok(unsafe { self.drvdata_unchecked() })
248	}	326	}
249	}	327	}
250		328