Re: [PATCH v3 02/11] gpu: nova-core: vbios: limit `BitToken` entry reads

Alexandre Courbot
2026-04-29T13:35:37Z

On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> If `header.token_size` is smaller than `BitToken`, then we currently can
> read past the end of `image.base.data`. Check that the token size is at
> least as big as `BitToken`.
>
> Fixes: dc70c6ae2441 ("gpu: nova-core: vbios: Add support to look up PMU table in FWSEC")
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> drivers/gpu/nova-core/vbios.rs | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 6de7e58e0da0..de856000de23 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -423,31 +423,31 @@ impl BitToken {
> /// Find a BIT token entry by BIT ID in a PciAtBiosImage
> fn from_id(image: &PciAtBiosImage, token_id: u8) -> Result<Self> {
> let header = &image.bit_header;
> + let entry_size = usize::from(header.token_size);
> +
> + if entry_size < size_of::<BitToken>() {
> + return Err(EINVAL);
> + }

You can get rid of this check if you convert the code as suggested
below.

>
> // Offset to the first token entry
> let tokens_start = image.bit_offset + usize::from(header.header_size);
>
> for i in 0..usize::from(header.token_entries) {
> - let entry_offset = tokens_start + (i * usize::from(header.token_size));
> -
> - // Make sure we don't go out of bounds
> - if entry_offset + usize::from(header.token_size) > image.base.data.len() {
> - return Err(EINVAL);
> - }
> + let entry_offset = tokens_start + (i * entry_size);

Should we use checked arithmetic here?

> + let entry = image
> + .base
> + .data
> + .get(entry_offset..)
> + .and_then(|data| data.get(..entry_size))
> + .ok_or(EINVAL)?;
>
> // Check if this token has the requested ID
> - if image.base.data[entry_offset] == token_id {
> + if entry[0] == token_id {
> return Ok(BitToken {
> - id: image.base.data[entry_offset],
> - data_version: image.base.data[entry_offset + 1],
> - data_size: u16::from_le_bytes([
> - image.base.data[entry_offset + 2],
> - image.base.data[entry_offset + 3],
> - ]),
> - data_offset: u16::from_le_bytes([
> - image.base.data[entry_offset + 4],
> - image.base.data[entry_offset + 5],
> - ]),
> + id: entry[0],
> + data_version: entry[1],
> + data_size: u16::from_le_bytes([entry[2], entry[3]]),
> + data_offset: u16::from_le_bytes([entry[4], entry[5]]),

A common pattern in this file (with several such sites still to fix), is
that since Nova only supports little-endian we can leverage `FromBytes`
in order to avoid all these `from_le_bytes` call. Here this would look
as follows:

for i in 0..usize::from(header.token_entries) {
let entry_offset = i
.checked_mul(entry_size)
.and_then(|off| tokens_start.checked_add(off))
.ok_or(EINVAL)?;

let entry = image
.base
.data
.get(entry_offset..entry_offset + entry_size)
.and_then(|data| data.get(..entry_size))
.ok_or(EINVAL)?;

let (token, _) = BitToken::from_bytes_copy_prefix(entry).ok_or(EINVAL)?;

if token.id == token_id {
return Ok(token);
}
}

which has several benefits:

- No error-prone `entry[index]` accesses,
- The size check on `entry_size` is done for free by
`from_bytes_copy_prefix`, and the slice bounds cannot be wrong,
- Shorter, more readable code overall.

Unfortunately we cannot just use `from_bytes_prefix` because we don't
have any alignment guarantee, but this is still an improvement IMHO.

If you go that way and derive `FromBytes` on `BitToken`, don't forget to
also make it `#[repr(C)]`. :)