From de6582833db0e695ba0c548e3cc2ad7dbb6aa260 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Tue, 18 Jun 2024 17:48:35 +0200 Subject: rust: add firmware abstractions Add an abstraction around the kernels firmware API to request firmware images. The abstraction provides functions to access the firmware's size and backing buffer. The firmware is released once the abstraction instance is dropped. Signed-off-by: Danilo Krummrich Acked-by: Boqun Feng Link: https://lore.kernel.org/r/20240618154841.6716-3-dakr@redhat.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/firmware.rs | 101 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 rust/kernel/firmware.rs (limited to 'rust/kernel/firmware.rs') diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs new file mode 100644 index 000000000000..b55ea1b45368 --- /dev/null +++ b/rust/kernel/firmware.rs @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Firmware abstraction +//! +//! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h") + +use crate::{bindings, device::Device, error::Error, error::Result, str::CStr}; +use core::ptr::NonNull; + +// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`, +// `firmware_request_platform`, `bindings::request_firmware_direct` +type FwFunc = + unsafe extern "C" fn(*mut *const bindings::firmware, *const i8, *mut bindings::device) -> i32; + +/// Abstraction around a C `struct firmware`. +/// +/// This is a simple abstraction around the C firmware API. Just like with the C API, firmware can +/// be requested. Once requested the abstraction provides direct access to the firmware buffer as +/// `&[u8]`. The firmware is released once [`Firmware`] is dropped. +/// +/// # Invariants +/// +/// The pointer is valid, and has ownership over the instance of `struct firmware`. +/// +/// Once requested, the `Firmware` backing buffer is not modified until it is freed when `Firmware` +/// is dropped. +/// +/// # Examples +/// +/// ``` +/// # use kernel::{c_str, device::Device, firmware::Firmware}; +/// +/// # // SAFETY: *NOT* safe, just for the example to get an `ARef` instance +/// # let dev = unsafe { Device::from_raw(core::ptr::null_mut()) }; +/// +/// let fw = Firmware::request(c_str!("path/to/firmware.bin"), &dev).unwrap(); +/// let blob = fw.data(); +/// ``` +pub struct Firmware(NonNull); + +impl Firmware { + fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result { + let mut fw: *mut bindings::firmware = core::ptr::null_mut(); + let pfw: *mut *mut bindings::firmware = &mut fw; + + // SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer. + // `name` and `dev` are valid as by their type invariants. + let ret = unsafe { func(pfw as _, name.as_char_ptr(), dev.as_raw()) }; + if ret != 0 { + return Err(Error::from_errno(ret)); + } + + // SAFETY: `func` not bailing out with a non-zero error code, guarantees that `fw` is a + // valid pointer to `bindings::firmware`. + Ok(Firmware(unsafe { NonNull::new_unchecked(fw) })) + } + + /// Send a firmware request and wait for it. See also `bindings::request_firmware`. + pub fn request(name: &CStr, dev: &Device) -> Result { + Self::request_internal(name, dev, bindings::request_firmware) + } + + /// Send a request for an optional firmware module. See also + /// `bindings::firmware_request_nowarn`. + pub fn request_nowarn(name: &CStr, dev: &Device) -> Result { + Self::request_internal(name, dev, bindings::firmware_request_nowarn) + } + + fn as_raw(&self) -> *mut bindings::firmware { + self.0.as_ptr() + } + + /// Returns the size of the requested firmware in bytes. + pub fn size(&self) -> usize { + // SAFETY: Safe by the type invariant. + unsafe { (*self.as_raw()).size } + } + + /// Returns the requested firmware as `&[u8]`. + pub fn data(&self) -> &[u8] { + // SAFETY: Safe by the type invariant. Additionally, `bindings::firmware` guarantees, if + // successfully requested, that `bindings::firmware::data` has a size of + // `bindings::firmware::size` bytes. + unsafe { core::slice::from_raw_parts((*self.as_raw()).data, self.size()) } + } +} + +impl Drop for Firmware { + fn drop(&mut self) { + // SAFETY: Safe by the type invariant. + unsafe { bindings::release_firmware(self.as_raw()) }; + } +} + +// SAFETY: `Firmware` only holds a pointer to a C `struct firmware`, which is safe to be used from +// any thread. +unsafe impl Send for Firmware {} + +// SAFETY: `Firmware` only holds a pointer to a C `struct firmware`, references to which are safe to +// be used from any thread. +unsafe impl Sync for Firmware {} -- cgit v1.2.3 From bbe98f4fde5a52aa01a1e1d754e1398228815fb0 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 19 Jun 2024 15:20:12 +0200 Subject: firmware: rust: improve safety comments Improve the wording of safety comments to be more explicit about what exactly is guaranteed to be valid. Suggested-by: Benno Lossin Signed-off-by: Danilo Krummrich Link: https://lore.kernel.org/r/20240619132029.59296-1-dakr@redhat.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/firmware.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'rust/kernel/firmware.rs') diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs index b55ea1b45368..386c8fb44785 100644 --- a/rust/kernel/firmware.rs +++ b/rust/kernel/firmware.rs @@ -22,8 +22,7 @@ type FwFunc = /// /// The pointer is valid, and has ownership over the instance of `struct firmware`. /// -/// Once requested, the `Firmware` backing buffer is not modified until it is freed when `Firmware` -/// is dropped. +/// The `Firmware`'s backing buffer is not modified. /// /// # Examples /// @@ -72,22 +71,22 @@ impl Firmware { /// Returns the size of the requested firmware in bytes. pub fn size(&self) -> usize { - // SAFETY: Safe by the type invariant. + // SAFETY: `self.as_raw()` is valid by the type invariant. unsafe { (*self.as_raw()).size } } /// Returns the requested firmware as `&[u8]`. pub fn data(&self) -> &[u8] { - // SAFETY: Safe by the type invariant. Additionally, `bindings::firmware` guarantees, if - // successfully requested, that `bindings::firmware::data` has a size of - // `bindings::firmware::size` bytes. + // SAFETY: `self.as_raw()` is valid by the type invariant. Additionally, + // `bindings::firmware` guarantees, if successfully requested, that + // `bindings::firmware::data` has a size of `bindings::firmware::size` bytes. unsafe { core::slice::from_raw_parts((*self.as_raw()).data, self.size()) } } } impl Drop for Firmware { fn drop(&mut self) { - // SAFETY: Safe by the type invariant. + // SAFETY: `self.as_raw()` is valid by the type invariant. unsafe { bindings::release_firmware(self.as_raw()) }; } } -- cgit v1.2.3 From 2c61b8c51d21d1b10c2881aa9c9918ff49f6fb7d Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 8 Jul 2024 22:07:20 +0200 Subject: firmware_loader: annotate doctests as `no_run` The doctests of `Firmware` are compile-time only tests, since they require a proper `Device` and a valid path to a (firmware) blob in order to do something sane on runtime - we can't satisfy both of those requirements. Hence, configure the example as `no_run`. Unfortunately, the kernel's Rust build system can't consider the `no_run` attribute yet. Hence, for the meantime, wrap the example code into a new function and never actually call it. Fixes: de6582833db0 ("rust: add firmware abstractions") Signed-off-by: Danilo Krummrich Link: https://lore.kernel.org/r/20240708200724.3203-1-dakr@redhat.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/firmware.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'rust/kernel/firmware.rs') diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs index 386c8fb44785..106a928a535e 100644 --- a/rust/kernel/firmware.rs +++ b/rust/kernel/firmware.rs @@ -26,14 +26,18 @@ type FwFunc = /// /// # Examples /// -/// ``` +/// ```no_run /// # use kernel::{c_str, device::Device, firmware::Firmware}; /// +/// # fn no_run() -> Result<(), Error> { /// # // SAFETY: *NOT* safe, just for the example to get an `ARef` instance /// # let dev = unsafe { Device::from_raw(core::ptr::null_mut()) }; /// -/// let fw = Firmware::request(c_str!("path/to/firmware.bin"), &dev).unwrap(); +/// let fw = Firmware::request(c_str!("path/to/firmware.bin"), &dev)?; /// let blob = fw.data(); +/// +/// # Ok(()) +/// # } /// ``` pub struct Firmware(NonNull); -- cgit v1.2.3 From a23b018c3bf646274f02edd46bf448c20c826d94 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 8 Jul 2024 22:07:21 +0200 Subject: firmware_loader: fix soundness issue in `request_internal` `request_internal` must be called with one of the following function pointers: request_firmware(), firmware_request_nowarn(), firmware_request_platform() or request_firmware_direct(). The previous `FwFunc` alias did not guarantee this, which is unsound. In order to fix this up, implement `FwFunc` as new type with a corresponding type invariant. Reported-by: Gary Guo Closes: https://lore.kernel.org/lkml/20240620143611.7995e0bb@eugeo/ Signed-off-by: Danilo Krummrich Reviewed-by: Christian Schrefl Link: https://lore.kernel.org/r/20240708200724.3203-2-dakr@redhat.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/firmware.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) (limited to 'rust/kernel/firmware.rs') diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs index 106a928a535e..2ba03af9f036 100644 --- a/rust/kernel/firmware.rs +++ b/rust/kernel/firmware.rs @@ -7,10 +7,23 @@ use crate::{bindings, device::Device, error::Error, error::Result, str::CStr}; use core::ptr::NonNull; -// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`, -// `firmware_request_platform`, `bindings::request_firmware_direct` -type FwFunc = - unsafe extern "C" fn(*mut *const bindings::firmware, *const i8, *mut bindings::device) -> i32; +/// # Invariants +/// +/// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`, +/// `bindings::firmware_request_platform`, `bindings::request_firmware_direct`. +struct FwFunc( + unsafe extern "C" fn(*mut *const bindings::firmware, *const i8, *mut bindings::device) -> i32, +); + +impl FwFunc { + fn request() -> Self { + Self(bindings::request_firmware) + } + + fn request_nowarn() -> Self { + Self(bindings::firmware_request_nowarn) + } +} /// Abstraction around a C `struct firmware`. /// @@ -48,7 +61,7 @@ impl Firmware { // SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer. // `name` and `dev` are valid as by their type invariants. - let ret = unsafe { func(pfw as _, name.as_char_ptr(), dev.as_raw()) }; + let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) }; if ret != 0 { return Err(Error::from_errno(ret)); } @@ -60,13 +73,13 @@ impl Firmware { /// Send a firmware request and wait for it. See also `bindings::request_firmware`. pub fn request(name: &CStr, dev: &Device) -> Result { - Self::request_internal(name, dev, bindings::request_firmware) + Self::request_internal(name, dev, FwFunc::request()) } /// Send a request for an optional firmware module. See also /// `bindings::firmware_request_nowarn`. pub fn request_nowarn(name: &CStr, dev: &Device) -> Result { - Self::request_internal(name, dev, bindings::firmware_request_nowarn) + Self::request_internal(name, dev, FwFunc::request_nowarn()) } fn as_raw(&self) -> *mut bindings::firmware { -- cgit v1.2.3