summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDanilo Krummrich <dakr@kernel.org>2025-01-03 17:46:03 +0100
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2025-01-07 11:31:45 +0100
commite1a51c2bf4b3b20868a0e6e9520b11639bd363f1 (patch)
tree2a1be8a3b07303bd8432fabb28e6e0ee09fd8661
parent9b880189327b9727640147253f3236ec5b3f704f (diff)
rust: driver: address soundness issue in `RegistrationOps`
The `RegistrationOps` trait holds some obligations to the caller and implementers. While being documented, the trait and the corresponding functions haven't been marked as unsafe. Hence, markt the trait and functions unsafe and add the corresponding safety comments. This patch does not include any fuctional changes. Reported-by: Gary Guo <gary@garyguo.net> Closes: https://lore.kernel.org/rust-for-linux/20241224195821.3b43302b.gary@garyguo.net/ Signed-off-by: Danilo Krummrich <dakr@kernel.org> Reviewed-by: Gary Guo <gary@garyguo.net> Link: https://lore.kernel.org/r/20250103164655.96590-4-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--rust/kernel/driver.rs25
-rw-r--r--rust/kernel/pci.rs8
-rw-r--r--rust/kernel/platform.rs8
3 files changed, 30 insertions, 11 deletions
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index c630e65098ed..2a16d5e64e6c 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -17,23 +17,35 @@ use macros::{pin_data, pinned_drop};
/// For instance, the PCI subsystem would set `RegType` to `bindings::pci_driver` and call
/// `bindings::__pci_register_driver` from `RegistrationOps::register` and
/// `bindings::pci_unregister_driver` from `RegistrationOps::unregister`.
-pub trait RegistrationOps {
+///
+/// # Safety
+///
+/// A call to [`RegistrationOps::unregister`] for a given instance of `RegType` is only valid if a
+/// preceding call to [`RegistrationOps::register`] has been successful.
+pub unsafe trait RegistrationOps {
/// The type that holds information about the registration. This is typically a struct defined
/// by the C portion of the kernel.
type RegType: Default;
/// Registers a driver.
///
+ /// # Safety
+ ///
/// On success, `reg` must remain pinned and valid until the matching call to
/// [`RegistrationOps::unregister`].
- fn register(
+ unsafe fn register(
reg: &Opaque<Self::RegType>,
name: &'static CStr,
module: &'static ThisModule,
) -> Result;
/// Unregisters a driver previously registered with [`RegistrationOps::register`].
- fn unregister(reg: &Opaque<Self::RegType>);
+ ///
+ /// # Safety
+ ///
+ /// Must only be called after a preceding successful call to [`RegistrationOps::register`] for
+ /// the same `reg`.
+ unsafe fn unregister(reg: &Opaque<Self::RegType>);
}
/// A [`Registration`] is a generic type that represents the registration of some driver type (e.g.
@@ -68,7 +80,8 @@ impl<T: RegistrationOps> Registration<T> {
// just been initialised above, so it's also valid for read.
let drv = unsafe { &*(ptr as *const Opaque<T::RegType>) };
- T::register(drv, name, module)
+ // SAFETY: `drv` is guaranteed to be pinned until `T::unregister`.
+ unsafe { T::register(drv, name, module) }
}),
})
}
@@ -77,7 +90,9 @@ impl<T: RegistrationOps> Registration<T> {
#[pinned_drop]
impl<T: RegistrationOps> PinnedDrop for Registration<T> {
fn drop(self: Pin<&mut Self>) {
- T::unregister(&self.reg);
+ // SAFETY: The existence of `self` guarantees that `self.reg` has previously been
+ // successfully registered with `T::register`
+ unsafe { T::unregister(&self.reg) };
}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index d5e7f0b15303..4c98b5b9aa1e 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -23,10 +23,12 @@ use kernel::prelude::*;
/// An adapter for the registration of PCI drivers.
pub struct Adapter<T: Driver>(T);
-impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
type RegType = bindings::pci_driver;
- fn register(
+ unsafe fn register(
pdrv: &Opaque<Self::RegType>,
name: &'static CStr,
module: &'static ThisModule,
@@ -45,7 +47,7 @@ impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
})
}
- fn unregister(pdrv: &Opaque<Self::RegType>) {
+ unsafe fn unregister(pdrv: &Opaque<Self::RegType>) {
// SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
unsafe { bindings::pci_unregister_driver(pdrv.get()) }
}
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 03287794f9d0..50e6b0421813 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -19,10 +19,12 @@ use core::ptr::addr_of_mut;
/// An adapter for the registration of platform drivers.
pub struct Adapter<T: Driver>(T);
-impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
type RegType = bindings::platform_driver;
- fn register(
+ unsafe fn register(
pdrv: &Opaque<Self::RegType>,
name: &'static CStr,
module: &'static ThisModule,
@@ -44,7 +46,7 @@ impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), module.0) })
}
- fn unregister(pdrv: &Opaque<Self::RegType>) {
+ unsafe fn unregister(pdrv: &Opaque<Self::RegType>) {
// SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
unsafe { bindings::platform_driver_unregister(pdrv.get()) };
}