linux-kernel - [PATCH v2 5/5] rust: firmware: implement `Ownable` for Firmware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241022224832.1505432-6-abdiel.janulgue@gmail.com>
Date: Wed, 23 Oct 2024 01:44:49 +0300
From: Abdiel Janulgue <abdiel.janulgue@...il.com>
To: rust-for-linux@...r.kernel.org,
	aliceryhl@...gle.com
Cc: dakr@...hat.com,
	linux-kernel@...r.kernel.org,
	airlied@...hat.com,
	miguel.ojeda.sandonis@...il.com,
	boqun.feng@...il.com,
	Abdiel Janulgue <abdiel.janulgue@...il.com>
Subject: [PATCH v2 5/5] rust: firmware: implement `Ownable` for Firmware

For consistency, wrap the firmware as an `Owned` smart pointer in the
constructor.

Cc: Danilo Krummrich <dakr@...hat.com>
Suggested-by: Boqun Feng <boqun.feng@...il.com>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@...il.com>
---
 rust/kernel/firmware.rs | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index dee5b4b18aec..6da834b37455 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -4,8 +4,8 @@
 //!
 //! 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;
+use crate::{bindings, device::Device, error::Error, error::Result, str::CStr,
+            types::{Opaque, Owned, Ownable}};
 
 /// # Invariants
 ///
@@ -52,10 +52,11 @@ fn request_nowarn() -> Self {
 /// # Ok(())
 /// # }
 /// ```
-pub struct Firmware(NonNull<bindings::firmware>);
+ #[repr(transparent)]
+pub struct Firmware(Opaque<bindings::firmware>);
 
 impl Firmware {
-    fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result<Self> {
+    fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result<Owned<Self>> {
         let mut fw: *mut bindings::firmware = core::ptr::null_mut();
         let pfw: *mut *mut bindings::firmware = &mut fw;
 
@@ -65,25 +66,26 @@ fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result<Self> {
         if ret != 0 {
             return Err(Error::from_errno(ret));
         }
-
+        // CAST: Self` is a `repr(transparent)` wrapper around `bindings::firmware`.
+        let ptr = fw.cast::<Self>();
         // 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) }))
+        Ok(unsafe { Owned::to_owned(ptr) })
     }
 
     /// 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<Owned<Self>> {
         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> {
+    pub fn request_nowarn(name: &CStr, dev: &Device) -> Result<Owned<Self>> {
         Self::request_internal(name, dev, FwFunc::request_nowarn())
     }
 
     fn as_raw(&self) -> *mut bindings::firmware {
-        self.0.as_ptr()
+        self.0.get()
     }
 
     /// Returns the size of the requested firmware in bytes.
@@ -101,10 +103,13 @@ pub fn data(&self) -> &[u8] {
     }
 }
 
-impl Drop for Firmware {
-    fn drop(&mut self) {
-        // SAFETY: `self.as_raw()` is valid by the type invariant.
-        unsafe { bindings::release_firmware(self.as_raw()) };
+unsafe impl Ownable for Firmware {
+    unsafe fn ptr_drop(ptr: *mut Self) {
+        // SAFETY:
+        // - By the type invariants, we have ownership of the ptr and can free it.
+        // - Per function safety, this is called in Owned::drop(), so `ptr` is a
+        //   unique pointer to object, it's safe to release the firmware.
+        unsafe { bindings::release_firmware(ptr.cast()) };
     }
 }
 
-- 
2.43.0