From a23b018c3bf646274f02edd46bf448c20c826d94 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 8 Jul 2024 22:07:21 +0200 Subject: [PATCH] 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(-) 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 {