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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked slice indexing using a firmware-derived offset can panic and crash the kernel.
- [High] Unchecked subtraction on untrusted firmware data can underflow and panic.
--

--- Patch [4]: [PATCH v2 04/11] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header` ---
commit 90813931dc3fe4cfc37444ced769050fbff9b8d3
Author: Eliot Courtney <[email protected]>

gpu: nova-core: vbios: use checked access in FwSecBiosImage::header

This patch safely handles firmware-derived values in FwSecBiosImage::header
by using checked access .get() instead of direct slice indexing.

> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 632c8a90ea765..bc752d135cbfd 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
[ ... ]

drivers/gpu/nova-core/vbios.rs:FwSecBiosBuilder::setup_falcon_data() {
    ...
        if pmu_in_first_fwsec {
            self.pmu_lookup_table = Some(PmuLookupTable::new(
                &self.base.dev,
                &first_fwsec.base.data[offset..],
            )?);
        } else {
            self.pmu_lookup_table = Some(PmuLookupTable::new(
                &self.base.dev,
                &self.base.data[offset..],
            )?);
        }
    ...
}

[Severity: High]
This is a pre-existing issue, but since this patch addresses unchecked
firmware accesses, I noticed these slice operations might lack upper bounds
checks. If a malformed firmware provides a large, out-of-bounds pointer,
could the offset exceed the length of the vector and cause a panic?


drivers/gpu/nova-core/vbios.rs:FwSecBiosBuilder::setup_falcon_data() {
    ...
            Ok(entry) => {
                let mut ucode_offset = usize::from_safe_cast(entry.data);
                ucode_offset -= pci_at_image.base.data.len();
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for this
subtraction on untrusted firmware data to underflow? If entry.data is
smaller than the PCI-AT image length, could this cause a panic or lead to
out-of-bounds accesses later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4