Re: [PATCH v3 04/11] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`

[Eliot Courtney]

On Wed Apr 29, 2026 at 10:56 PM JST, Alexandre Courbot wrote:
> On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
>> Use checked access in `FwSecBiosImage::header` for getting the header
>> version since the value is firmware derived.
>>
>> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
>> Reviewed-by: Joel Fernandes <joelagnelf@xxxxxxxxxx>
>> Signed-off-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
>> ---
>>  drivers/gpu/nova-core/vbios.rs | 17 +++++++----------
>>  1 file changed, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>> index 632c8a90ea76..bc752d135cbf 100644
>> --- a/drivers/gpu/nova-core/vbios.rs
>> +++ b/drivers/gpu/nova-core/vbios.rs
>> @@ -996,17 +996,14 @@ fn build(self) -> Result<FwSecBiosImage> {
>>  impl FwSecBiosImage {
>>      /// Get the FwSec header ([`FalconUCodeDesc`]).
>>      pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
>> -        // Get the falcon ucode offset that was found in setup_falcon_data.
>> -        let falcon_ucode_offset = self.falcon_ucode_offset;
>> +        let data = self
>> +            .base
>> +            .data
>> +            .get(self.falcon_ucode_offset..)
>> +            .ok_or(EINVAL)?;
>>  
>> -        // Read the first 4 bytes to get the version.
>> -        let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
>> -            .try_into()
>> -            .map_err(|_| EINVAL)?;
>> -        let hdr = u32::from_le_bytes(hdr_bytes);
>> -        let ver = (hdr & 0xff00) >> 8;
>> -
>> -        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
>> +        // Read the version byte from the header.
>> +        let ver = data.get(1).copied().ok_or(EINVAL)?;
>
> This doesn't need to be done with this patch, but once the kernel-wide
> `bitfield` macro lands I hope we can use to to define a proper header
> type for this and use it both here and in `firmware.rs` which does some
> nasty bit masking - see for instance `FalconUCodeDescriptor::size`.

Yerp that will be nice. For this specific instance, not sure if it's
worth it though since it's just get the value of one byte.