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

["Danilo Krummrich" <dakr@kernel.org>]

On Tue Jun 30, 2026 at 9:47 PM CEST, Timur Tabi wrote:
> +/// 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.
> +/// See also `bindings::request_firmware_into_buf`.
> +///
> +/// 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 {
> +    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(),

Sashiko's concern about buf being an empty slice, despite being nonsensical,
seems valid. The allocated_size field in struct fw_priv, if set to zero, is
interpreted as "the driver did not provide a buffer" and hence the firmware
loader assumes that it has to treat the data pointer as a self-allocated buffer.
In the case of passing an empty slice, this would be a dangling pointer.

> +            buf.len(),
> +        )
> +    };
> +    if ret != 0 {
> +        return Err(Error::from_errno(ret));
> +    }
> +
> +    // 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 {}
> -- 
> 2.54.0