Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH] rust: firmware: return empty slice for zero-size firmware
@ 2026-06-05  4:11 Yuan Tan
  2026-06-05  7:10 ` Onur Özkan
  0 siblings, 1 reply; 7+ messages in thread
From: Yuan Tan @ 2026-06-05  4:11 UTC (permalink / raw)
  To: ojeda, boqun, rust-for-linux
  Cc: zhiyunq, ardalan, pgovind2, dzueck, Yuan Tan, stable

Firmware::data() builds a Rust slice with core::slice::from_raw_parts().
Unlike many C APIs, from_raw_parts() requires its pointer argument to be
non-NULL even when the length is zero.

The firmware loader can represent an empty firmware image with size == 0
and data == NULL. Passing that pointer to from_raw_parts() would be
undefined behavior.

Return an empty slice before constructing the raw slice. For non-zero
firmware sizes, the existing firmware API guarantee that data has size
bytes also means that the pointer is non-NULL.

Fixes: de6582833db0 ("rust: add firmware abstractions")
Cc: stable@vger.kernel.org
Reported-by: Priya Bala Govindasamy <pgovind2@uci.edu>
Reported-by: Dylan Zueck <dzueck@uci.edu>
Signed-off-by: Yuan Tan <ytan089@ucr.edu>
---
 rust/kernel/firmware.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index 71168d8004e2..5e22a574a91e 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -106,10 +106,17 @@ pub fn size(&self) -> usize {
 
     /// Returns the requested firmware as `&[u8]`.
     pub fn data(&self) -> &[u8] {
+        let size = self.size();
+
+        if size == 0 {
+            return &[];
+        }
+
         // SAFETY: `self.as_raw()` is valid by the type invariant. Additionally,
         // `bindings::firmware` guarantees, if successfully requested, that
         // `bindings::firmware::data` has a size of `bindings::firmware::size` bytes.
-        unsafe { core::slice::from_raw_parts((*self.as_raw()).data, self.size()) }
+        // For non-zero `size`, this also means `bindings::firmware::data` is not NULL.
+        unsafe { core::slice::from_raw_parts((*self.as_raw()).data, size) }
     }
 }
 
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: firmware: return empty slice for zero-size firmware
  2026-06-05  4:11 [PATCH] rust: firmware: return empty slice for zero-size firmware Yuan Tan
@ 2026-06-05  7:10 ` Onur Özkan
  2026-06-05  8:13   ` Gary Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Onur Özkan @ 2026-06-05  7:10 UTC (permalink / raw)
  To: Yuan Tan
  Cc: ojeda, boqun, rust-for-linux, zhiyunq, ardalan, pgovind2, dzueck,
	stable

On Thu, 04 Jun 2026 21:11:34 -0700
Yuan Tan <ytan089@ucr.edu> wrote:

> Firmware::data() builds a Rust slice with core::slice::from_raw_parts().
> Unlike many C APIs, from_raw_parts() requires its pointer argument to be
> non-NULL even when the length is zero.
> 
> The firmware loader can represent an empty firmware image with size == 0


I haven't checked in detail yet but "empty firmware image with size == 0"
sounds like an invalid image. Can such an image actually make it all the way
to Firmware::data()? I would be surprised if the loader accepted it.

Thanks,
Onur

> and data == NULL. Passing that pointer to from_raw_parts() would be
> undefined behavior.
> 
> Return an empty slice before constructing the raw slice. For non-zero
> firmware sizes, the existing firmware API guarantee that data has size
> bytes also means that the pointer is non-NULL.
> 
> Fixes: de6582833db0 ("rust: add firmware abstractions")
> Cc: stable@vger.kernel.org
> Reported-by: Priya Bala Govindasamy <pgovind2@uci.edu>
> Reported-by: Dylan Zueck <dzueck@uci.edu>
> Signed-off-by: Yuan Tan <ytan089@ucr.edu>
> ---
>  rust/kernel/firmware.rs | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
> index 71168d8004e2..5e22a574a91e 100644
> --- a/rust/kernel/firmware.rs
> +++ b/rust/kernel/firmware.rs
> @@ -106,10 +106,17 @@ pub fn size(&self) -> usize {
>  
>      /// Returns the requested firmware as `&[u8]`.
>      pub fn data(&self) -> &[u8] {
> +        let size = self.size();
> +
> +        if size == 0 {
> +            return &[];
> +        }
> +
>          // SAFETY: `self.as_raw()` is valid by the type invariant. Additionally,
>          // `bindings::firmware` guarantees, if successfully requested, that
>          // `bindings::firmware::data` has a size of `bindings::firmware::size` bytes.
> -        unsafe { core::slice::from_raw_parts((*self.as_raw()).data, self.size()) }
> +        // For non-zero `size`, this also means `bindings::firmware::data` is not NULL.
> +        unsafe { core::slice::from_raw_parts((*self.as_raw()).data, size) }
>      }
>  }
>  
> -- 
> 2.43.2
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: firmware: return empty slice for zero-size firmware
  2026-06-05  7:10 ` Onur Özkan
@ 2026-06-05  8:13   ` Gary Guo
  2026-06-05  9:16     ` Onur Özkan
  0 siblings, 1 reply; 7+ messages in thread
From: Gary Guo @ 2026-06-05  8:13 UTC (permalink / raw)
  To: Onur Özkan, Yuan Tan
  Cc: ojeda, boqun, rust-for-linux, zhiyunq, ardalan, pgovind2, dzueck,
	stable

On Fri Jun 5, 2026 at 8:10 AM BST, Onur Özkan wrote:
> On Thu, 04 Jun 2026 21:11:34 -0700
> Yuan Tan <ytan089@ucr.edu> wrote:
>
>> Firmware::data() builds a Rust slice with core::slice::from_raw_parts().
>> Unlike many C APIs, from_raw_parts() requires its pointer argument to be
>> non-NULL even when the length is zero.
>> 
>> The firmware loader can represent an empty firmware image with size == 0
>
>
> I haven't checked in detail yet but "empty firmware image with size == 0"
> sounds like an invalid image. Can such an image actually make it all the way
> to Firmware::data()? I would be surprised if the loader accepted it.

`kernel_read_file` will return EINVAL if file is of zero size. But I think the
decompression path might produce this? The zstd code just does a

    out_buf = vzalloc(out_size);

which will trigger a WARN but still return NULL.

That said, I doubt any valid firmware will be just a compressed empty file.

Best,
Gary

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: firmware: return empty slice for zero-size firmware
  2026-06-05  8:13   ` Gary Guo
@ 2026-06-05  9:16     ` Onur Özkan
  2026-06-05  9:28       ` Gary Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Onur Özkan @ 2026-06-05  9:16 UTC (permalink / raw)
  To: Gary Guo
  Cc: Yuan Tan, ojeda, boqun, rust-for-linux, zhiyunq, ardalan,
	pgovind2, dzueck, stable

On Fri, 05 Jun 2026 09:13:55 +0100
Gary Guo <gary@garyguo.net> wrote:

> On Fri Jun 5, 2026 at 8:10 AM BST, Onur Özkan wrote:
> > On Thu, 04 Jun 2026 21:11:34 -0700
> > Yuan Tan <ytan089@ucr.edu> wrote:
> >
> >> Firmware::data() builds a Rust slice with core::slice::from_raw_parts().
> >> Unlike many C APIs, from_raw_parts() requires its pointer argument to be
> >> non-NULL even when the length is zero.
> >> 
> >> The firmware loader can represent an empty firmware image with size == 0
> >
> >
> > I haven't checked in detail yet but "empty firmware image with size == 0"
> > sounds like an invalid image. Can such an image actually make it all the way
> > to Firmware::data()? I would be surprised if the loader accepted it.
> 
> `kernel_read_file` will return EINVAL if file is of zero size. But I think the
> decompression path might produce this? The zstd code just does a
> 
>     out_buf = vzalloc(out_size);
> 
> which will trigger a WARN but still return NULL.

On NULL, it immediately fails with ENOMEM:

	out_buf = vzalloc(out_size);
	if (!out_buf)
		return -ENOMEM;

Thanks,
Onur

> 
> That said, I doubt any valid firmware will be just a compressed empty file.
> 
> Best,
> Gary

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: firmware: return empty slice for zero-size firmware
  2026-06-05  9:16     ` Onur Özkan
@ 2026-06-05  9:28       ` Gary Guo
  2026-06-05 11:15         ` Miguel Ojeda
  0 siblings, 1 reply; 7+ messages in thread
From: Gary Guo @ 2026-06-05  9:28 UTC (permalink / raw)
  To: Onur Özkan, Gary Guo
  Cc: Yuan Tan, ojeda, boqun, rust-for-linux, zhiyunq, ardalan,
	pgovind2, dzueck, stable

On Fri Jun 5, 2026 at 10:16 AM BST, Onur Özkan wrote:
> On Fri, 05 Jun 2026 09:13:55 +0100
> Gary Guo <gary@garyguo.net> wrote:
>
>> On Fri Jun 5, 2026 at 8:10 AM BST, Onur Özkan wrote:
>> > On Thu, 04 Jun 2026 21:11:34 -0700
>> > Yuan Tan <ytan089@ucr.edu> wrote:
>> >
>> >> Firmware::data() builds a Rust slice with core::slice::from_raw_parts().
>> >> Unlike many C APIs, from_raw_parts() requires its pointer argument to be
>> >> non-NULL even when the length is zero.
>> >> 
>> >> The firmware loader can represent an empty firmware image with size == 0
>> >
>> >
>> > I haven't checked in detail yet but "empty firmware image with size == 0"
>> > sounds like an invalid image. Can such an image actually make it all the way
>> > to Firmware::data()? I would be surprised if the loader accepted it.
>> 
>> `kernel_read_file` will return EINVAL if file is of zero size. But I think the
>> decompression path might produce this? The zstd code just does a
>> 
>>     out_buf = vzalloc(out_size);
>> 
>> which will trigger a WARN but still return NULL.
>
> On NULL, it immediately fails with ENOMEM:
>
> 	out_buf = vzalloc(out_size);
> 	if (!out_buf)
> 		return -ENOMEM;
>
> Thanks,
> Onur

Oh right. Arguably the wrong error code, but it does prevent the path from being
hit. xz decompression always grow at least 1 page and thus won't hit NULL case
as well.

So indeed under no paths we will have a sucessful `request_firmware` with
`buffer` is NULL.

Best,
Gary

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: firmware: return empty slice for zero-size firmware
  2026-06-05  9:28       ` Gary Guo
@ 2026-06-05 11:15         ` Miguel Ojeda
  2026-06-05 13:11           ` Onur Özkan
  0 siblings, 1 reply; 7+ messages in thread
From: Miguel Ojeda @ 2026-06-05 11:15 UTC (permalink / raw)
  To: Gary Guo
  Cc: Onur Özkan, Yuan Tan, ojeda, boqun, rust-for-linux, zhiyunq,
	ardalan, pgovind2, dzueck, stable

On Fri, Jun 5, 2026 at 11:28 AM Gary Guo <gary@garyguo.net> wrote:
>
> Oh right. Arguably the wrong error code, but it does prevent the path from being
> hit. xz decompression always grow at least 1 page and thus won't hit NULL case
> as well.
>
> So indeed under no paths we will have a sucessful `request_firmware` with
> `buffer` is NULL.

If we are not expecting it in practice, then at least a
`debug_assert!` would be nice, or perhaps even making it an invariant?

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rust: firmware: return empty slice for zero-size firmware
  2026-06-05 11:15         ` Miguel Ojeda
@ 2026-06-05 13:11           ` Onur Özkan
  0 siblings, 0 replies; 7+ messages in thread
From: Onur Özkan @ 2026-06-05 13:11 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Gary Guo, Yuan Tan, ojeda, boqun, rust-for-linux, zhiyunq,
	ardalan, pgovind2, dzueck, stable

On Fri, 05 Jun 2026 13:15:32 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Fri, Jun 5, 2026 at 11:28 AM Gary Guo <gary@garyguo.net> wrote:
> >
> > Oh right. Arguably the wrong error code, but it does prevent the path from being
> > hit. xz decompression always grow at least 1 page and thus won't hit NULL case
> > as well.
> >
> > So indeed under no paths we will have a sucessful `request_firmware` with
> > `buffer` is NULL.
> 
> If we are not expecting it in practice, then at least a
> `debug_assert!` would be nice, or perhaps even making it an invariant?

I think `debug_assert!` would be nice.

Thanks,
Onur

> 
> Cheers,
> Miguel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-05 13:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-05  4:11 [PATCH] rust: firmware: return empty slice for zero-size firmware Yuan Tan
2026-06-05  7:10 ` Onur Özkan
2026-06-05  8:13   ` Gary Guo
2026-06-05  9:16     ` Onur Özkan
2026-06-05  9:28       ` Gary Guo
2026-06-05 11:15         ` Miguel Ojeda
2026-06-05 13:11           ` Onur Özkan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox