Re: [PATCH v7 2/8] rust: firmware: add request_into_buf()

On Sat Aug 1, 2026 at 5:24 AM JST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issu=
e(s) to consider:
> - [High] `request_into_buf` discards the actual size of the loaded firmwa=
re, exposing the caller to trailing uninitialized or stale data.
> - [Low] The wrapper unnecessarily casts a mutable pointer to a const poin=
ter, which is unidiomatic and confusing.
> --
>
> --- Patch [2]: [PATCH v7 2/8] rust: firmware: add request_into_buf() ---
> commit e3d518ea065bd3aeb4a38939d026df9eef8651b7
> Author: Timur Tabi <[email protected]>
>
> rust: firmware: add request_into_buf()
>
> Add request_into_buf(), a Rust wrapper around the request_firmware_into_b=
uf()
> function. This variant loads the firmware image directly into a caller-pr=
ovided
> buffer rather than a kernel-allocated one.
>
>> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
>> index 1e3dd2e3422db..deb119d4813c0 100644
>> --- a/rust/kernel/firmware.rs
>> +++ b/rust/kernel/firmware.rs
>
> [ ... ]
>
>> @@ -116,6 +116,50 @@ fn drop(&mut self) {
>>      }
>>  }
>> =20
>> +/// Load firmware directly into the caller-provided `buf`.
>> +///
>> +/// On success the firmware image has been copied into `buf`; the calle=
r accesses the data
>> +/// through `buf` itself.
>> +///
>> +/// This is intentionally a stand-alone function rather than a `Firmwar=
e` constructor. For
>> +/// the `into_buf` path, the firmware data lives in the caller's `buf`,=
 not in a
>> +/// kernel-owned buffer, so returning a `Firmware` would expose `Firmwa=
re::data()` as a
>> +/// second handle aliasing `buf` (and `release_firmware()` does not fre=
e `buf` anyway).
>> +pub fn request_into_buf(name: &CStr, dev: &Device, buf: &mut [u8]) -> R=
esult {
>
> [ ... ]
>
>> +    let mut fw: *mut bindings::firmware =3D core::ptr::null_mut();
>> +    let pfw: *mut *mut bindings::firmware =3D &mut fw;
>> +    let pfw: *mut *const bindings::firmware =3D pfw.cast();
>
> [Severity: Low]
> This isn't a bug, but is it necessary to declare a mutable pointer and th=
en
> cast it to a pointer-to-const?=20
>
> The FFI function expects a *mut *const bindings::firmware. Declaring
> let mut fw: *const bindings::firmware =3D core::ptr::null(); would produc=
e
> the correct *mut *const address naturally, without requiring these casts.

Sashiko is correct here, this can be simplified as follows (on apply if
no respin is required?):

diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index 5b336bd1f09c..bf759c4f8996 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -139,16 +139,14 @@ pub fn request_into_buf(name: &CStr, dev: &Device, bu=
f: &mut [u8]) -> Result {
         return Err(EINVAL);
     }

-    let mut fw: *mut bindings::firmware =3D core::ptr::null_mut();
-    let pfw: *mut *mut bindings::firmware =3D &mut fw;
-    let pfw: *mut *const bindings::firmware =3D pfw.cast();
+    let mut fw: *const bindings::firmware =3D core::ptr::null();

-    // SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::f=
irmware` pointer.
+    // SAFETY: `&mut fw` is a valid pointer to a NULL initialized `binding=
s::firmware` pointer.
     // `name` and `dev` are valid as by their type invariants. `buf` is a =
valid writable
     // buffer of `buf.len()` bytes.
     to_result(unsafe {
         bindings::request_firmware_into_buf(
-            pfw,
+            &mut fw,
             name.as_char_ptr(),
             dev.as_raw(),
             buf.as_mut_ptr().cast(),