1

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 <gary@garyguo.net>
Closes: https://lore.kernel.org/lkml/20240620143611.7995e0bb@eugeo/
Signed-off-by: Danilo Krummrich <dakr@redhat.com>
Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Link: https://lore.kernel.org/r/20240708200724.3203-2-dakr@redhat.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Danilo Krummrich 2024-07-08 22:07:21 +02:00 committed by Greg Kroah-Hartman
parent 2c61b8c51d
commit a23b018c3b

View File

@ -7,10 +7,23 @@
use crate::{bindings, device::Device, error::Error, error::Result, str::CStr}; use crate::{bindings, device::Device, error::Error, error::Result, str::CStr};
use core::ptr::NonNull; use core::ptr::NonNull;
// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`, /// # Invariants
// `firmware_request_platform`, `bindings::request_firmware_direct` ///
type FwFunc = /// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`,
unsafe extern "C" fn(*mut *const bindings::firmware, *const i8, *mut bindings::device) -> i32; /// `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`. /// 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. // SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
// `name` and `dev` are valid as by their type invariants. // `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 { if ret != 0 {
return Err(Error::from_errno(ret)); 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`. /// Send a firmware request and wait for it. See also `bindings::request_firmware`.
pub fn request(name: &CStr, dev: &Device) -> Result<Self> { pub fn request(name: &CStr, dev: &Device) -> Result<Self> {
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 /// Send a request for an optional firmware module. See also
/// `bindings::firmware_request_nowarn`. /// `bindings::firmware_request_nowarn`.
pub fn request_nowarn(name: &CStr, dev: &Device) -> Result<Self> { pub fn request_nowarn(name: &CStr, dev: &Device) -> Result<Self> {
Self::request_internal(name, dev, bindings::firmware_request_nowarn) Self::request_internal(name, dev, FwFunc::request_nowarn())
} }
fn as_raw(&self) -> *mut bindings::firmware { fn as_raw(&self) -> *mut bindings::firmware {