Re: [PATCH 5/5] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`

[Eliot Courtney]

On Sat Apr 11, 2026 at 12:05 AM JST, Joel Fernandes wrote:
> Reviewed-by: Joel Fernandes <joelagnelf@xxxxxxxxxx> , one comment below
>
> On 4/10/2026 4:38 AM, Eliot Courtney wrote:
>> Use checked arithmetic and access for extracting the microcode since the
>> offsets are firmware derived.
>> 
>> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
>> Signed-off-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
>> ---
>>  drivers/gpu/nova-core/vbios.rs | 19 ++++++++++++-------
>>  1 file changed, 12 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>> index 3bd3ac3a69f2..b509cd8407a5 100644
>> --- a/drivers/gpu/nova-core/vbios.rs
>> +++ b/drivers/gpu/nova-core/vbios.rs
>> @@ -1027,16 +1027,21 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
>>  
>>      /// Get the ucode data as a byte slice
>>      pub(crate) fn ucode(&self, desc: &FalconUCodeDesc) -> Result<&[u8]> {
>> -        let falcon_ucode_offset = self.falcon_ucode_offset;
>> -
>>          // The ucode data follows the descriptor.
>> -        let ucode_data_offset = falcon_ucode_offset + desc.size();
>> -        let size = usize::from_safe_cast(desc.imem_load_size() + desc.dmem_load_size());
>> +        let data = self
>> +            .base
>> +            .data
>> +            .get(self.falcon_ucode_offset..)
>> +            .ok_or(ERANGE)?;
>> +        let size = usize::from_safe_cast(
>> +            desc.imem_load_size()
>> +                .checked_add(desc.dmem_load_size())
>> +                .ok_or(ERANGE)?,
>> +        );
>>  
>>          // Get the data slice, checking bounds in a single operation.
>> -        self.base
>> -            .data
>> -            .get(ucode_data_offset..ucode_data_offset + size)
>> +        data.get(desc.size()..)
>> +            .and_then(|data| data.get(..size))
>
> It might be worth adding something like:
>
> data.get_slice(start, size) -> Result
>
> in R4L longer term if the data.get(start..).and_then(|data|
> data.get(..size)) pattern is common. It seems to be so in this series.

Yeah, this sounds reasonable although I think we'd need a name that
makes it more obvious it takes a size. I had a look at rust standard
library code and it seems a common pattern is to use split_at_checked
for the initial split. Unfortunately we can't use that yet I think since
it's in 1.80.0, but maybe we can use it very soon.

I had a look and it looks like there aren't really other existing
patterns like .get(start..).and_then(.. get(..size)) in the rust kernel
code. And in nova-core other usages of getting start..start+size slices
have local proofs that start+size doesn't overflow.

For the most part in other code hopefully we have local proofs that
start..start+size won't overflow and we can just .get(start..start+size).