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>

authored by

Danilo Krummrich and committed by

Greg Kroah-Hartman 2 years ago e1a51c2b 9b880189

+30 -11

3 changed files

expand all

rust

kernel

driver.rs

pci.rs

platform.rs

+20 -5

rust/kernel/driver.rs

··· 17 17 /// For instance, the PCI subsystem would set `RegType` to `bindings::pci_driver` and call 18 18 /// `bindings::__pci_register_driver` from `RegistrationOps::register` and 19 19 /// `bindings::pci_unregister_driver` from `RegistrationOps::unregister`. 20 - pub trait RegistrationOps { 20 + /// 21 + /// # Safety 22 + /// 23 + /// A call to [`RegistrationOps::unregister`] for a given instance of `RegType` is only valid if a 24 + /// preceding call to [`RegistrationOps::register`] has been successful. 25 + pub unsafe trait RegistrationOps { 21 26 /// The type that holds information about the registration. This is typically a struct defined 22 27 /// by the C portion of the kernel. 23 28 type RegType: Default; 24 29 25 30 /// Registers a driver. 26 31 /// 32 + /// # Safety 33 + /// 27 34 /// On success, `reg` must remain pinned and valid until the matching call to 28 35 /// [`RegistrationOps::unregister`]. 29 - fn register( 36 + unsafe fn register( 30 37 reg: &Opaque<Self::RegType>, 31 38 name: &'static CStr, 32 39 module: &'static ThisModule, 33 40 ) -> Result; 34 41 35 42 /// Unregisters a driver previously registered with [`RegistrationOps::register`]. 36 - fn unregister(reg: &Opaque<Self::RegType>); 43 + /// 44 + /// # Safety 45 + /// 46 + /// Must only be called after a preceding successful call to [`RegistrationOps::register`] for 47 + /// the same `reg`. 48 + unsafe fn unregister(reg: &Opaque<Self::RegType>); 37 49 } 38 50 39 51 /// A [`Registration`] is a generic type that represents the registration of some driver type (e.g. ··· 80 68 // just been initialised above, so it's also valid for read. 81 69 let drv = unsafe { &*(ptr as *const Opaque<T::RegType>) }; 82 70 83 - T::register(drv, name, module) 71 + // SAFETY: `drv` is guaranteed to be pinned until `T::unregister`. 72 + unsafe { T::register(drv, name, module) } 84 73 }), 85 74 }) 86 75 } ··· 90 77 #[pinned_drop] 91 78 impl<T: RegistrationOps> PinnedDrop for Registration<T> { 92 79 fn drop(self: Pin<&mut Self>) { 93 - T::unregister(&self.reg); 80 + // SAFETY: The existence of `self` guarantees that `self.reg` has previously been 81 + // successfully registered with `T::register` 82 + unsafe { T::unregister(&self.reg) }; 94 83 } 95 84 } 96 85

+5 -3

rust/kernel/pci.rs

··· 23 23 /// An adapter for the registration of PCI drivers. 24 24 pub struct Adapter<T: Driver>(T); 25 25 26 - impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { 26 + // SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if 27 + // a preceding call to `register` has been successful. 28 + unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { 27 29 type RegType = bindings::pci_driver; 28 30 29 - fn register( 31 + unsafe fn register( 30 32 pdrv: &Opaque<Self::RegType>, 31 33 name: &'static CStr, 32 34 module: &'static ThisModule, ··· 47 45 }) 48 46 } 49 47 50 - fn unregister(pdrv: &Opaque<Self::RegType>) { 48 + unsafe fn unregister(pdrv: &Opaque<Self::RegType>) { 51 49 // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. 52 50 unsafe { bindings::pci_unregister_driver(pdrv.get()) } 53 51 }

+5 -3

rust/kernel/platform.rs

··· 19 19 /// An adapter for the registration of platform drivers. 20 20 pub struct Adapter<T: Driver>(T); 21 21 22 - impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { 22 + // SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if 23 + // a preceding call to `register` has been successful. 24 + unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { 23 25 type RegType = bindings::platform_driver; 24 26 25 - fn register( 27 + unsafe fn register( 26 28 pdrv: &Opaque<Self::RegType>, 27 29 name: &'static CStr, 28 30 module: &'static ThisModule, ··· 46 44 to_result(unsafe { bindings::__platform_driver_register(pdrv.get(), module.0) }) 47 45 } 48 46 49 - fn unregister(pdrv: &Opaque<Self::RegType>) { 47 + unsafe fn unregister(pdrv: &Opaque<Self::RegType>) { 50 48 // SAFETY: `pdrv` is guaranteed to be a valid `RegType`. 51 49 unsafe { bindings::platform_driver_unregister(pdrv.get()) }; 52 50 }

Configure Feed

Configure Feed