Re: [PATCH v3 1/7] rust: firmware: add request_into_buf()

[Alvin Sun <alvin.sun@linux.dev>]

On 7/3/26 03:27, Timur Tabi wrote:
> Add request_into_buf(), a Rust wrapper around the
> request_firmware_into_buf() function. This variant loads the firmware
> image directly into a caller-provided buffer rather than a
> kernel-allocated one.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>   rust/kernel/firmware.rs | 47 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>
> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
> index 71168d8004e2..4460fb2cd5d8 100644
> --- a/rust/kernel/firmware.rs
> +++ b/rust/kernel/firmware.rs
> @@ -120,6 +120,53 @@ fn drop(&mut self) {
>       }
>   }
>   
> +/// Load firmware directly into the caller-provided `buf`.
> +///
> +/// On success the firmware image has been copied into `buf`; the caller accesses the data
> +/// through `buf` itself.
> +///
> +/// This is intentionally a stand-alone function rather than a `Firmware` 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 `Firmware::data()` as a
> +/// second handle aliasing `buf` (and `release_firmware()` does not free `buf` anyway).
> +pub fn request_into_buf(name: &CStr, dev: &Device, buf: &mut [u8]) -> Result {
> +    // `as_mut_ptr()` on an empty slice returns a non-NULL pointer to
> +    // memory which the loader does not own. Passing that pointer with `size == 0`
> +    // makes the loader believe that it is buffer it allocated itself, so when
> +    // `release_firmware()` is called, it will vfree the pointer and trigger a
> +    // bug. Reject empty slices to avoid this situation.
> +    if buf.is_empty() {
> +        return Err(crate::error::code::EINVAL);

`EINVAL` is already in prelude, you can use it directly.

> +    }
> +
> +    let mut fw: *mut bindings::firmware = core::ptr::null_mut();
> +    let pfw: *mut *mut bindings::firmware = &mut fw;
> +    let pfw: *mut *const bindings::firmware = pfw.cast();
> +
> +    // SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
> +    // `name` and `dev` are valid as by their type invariants. `buf` is a valid writable
> +    // buffer of `buf.len()` bytes.
> +    let ret = unsafe {
> +        bindings::request_firmware_into_buf(
> +            pfw,
> +            name.as_char_ptr(),
> +            dev.as_raw(),
> +            buf.as_mut_ptr().cast(),
> +            buf.len(),
> +        )
> +    };
> +    if ret != 0 {
> +        return Err(Error::from_errno(ret));
> +    }

`to_result` can be used here to simplify.

> +
> +    // The firmware bytes are now in `buf`, which the caller owns, so we don't need
> +    // the kernel to hang on to it any more.
> +    // SAFETY: `fw` is a valid pointer returned by `request_firmware_into_buf`.
> +    unsafe { bindings::release_firmware(fw) };
> +
> +    Ok(())
> +}
> +
>   // 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 {}