From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: gpu: nova-core: vbios: use checked accesses in `setup_falcon_data` Date: Sun, 12 Apr 2026 10:18:46 +1000 Message-ID: In-Reply-To: <20260410-fix-vbios-v1-3-bc6f71d153d6@nvidia.com> References: <20260410-fix-vbios-v1-0-bc6f71d153d6@nvidia.com> <20260410-fix-vbios-v1-3-bc6f71d153d6@nvidia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review Two changes here: 1. **PMU lookup table data slicing**: The `pmu_in_first_fwsec` branch keeps= `&first_fwsec.base.data[offset..]` as unchecked (which is fine since `offs= et < first_fwsec.base.data.len()` is guaranteed by the `if` above), while t= he `else` branch is changed to use `.get(offset..).ok_or(EINVAL)?`. ```rust + let pmu_lookup_data =3D if pmu_in_first_fwsec { + &first_fwsec.base.data[offset..] } else { - self.pmu_lookup_table =3D Some(PmuLookupTable::new( - &self.base.dev, - &self.base.data[offset..], - )?); - } + self.base.data.get(offset..).ok_or(EINVAL)? + }; ``` This is a good refactor =E2=80=94 it also deduplicates the `PmuLookupTable:= :new` call. The asymmetry between the two branches (checked vs unchecked) i= s acceptable since the `pmu_in_first_fwsec` branch has the earlier `offset = < first_fwsec.base.data.len()` guard. However, for consistency, it might be= worth using `.get(offset..)` on both branches =E2=80=94 but this is stylis= tic, not a correctness issue. 2. **`ucode_offset` subtraction**: The old code did `ucode_offset -=3D pci_= at_image.base.data.len()` which could underflow (wrapping in release, panic= king in debug). The fix uses `checked_sub`: ```rust + let mut ucode_offset =3D usize::from_safe_cast(entry.data) + .checked_sub(pci_at_image.base.data.len()) + .ok_or(EINVAL)?; ``` This is correct. Note the same unchecked subtraction pattern exists 6 lines= earlier in the function (`offset -=3D pci_at_image.base.data.len()` at the= top of `setup_falcon_data`, line ~926 of the original). That subtraction i= s **not** fixed by this patch. This could be a follow-up, though the call c= ontext may provide guarantees =E2=80=94 worth mentioning to the author. **Reviewed-by assessment: Good, but the unchecked `offset -=3D pci_at_image= .base.data.len()` earlier in the same function (line ~926 in the original) = appears to have the same vulnerability and should probably also use `checke= d_sub`.** --- --- Generated by Claude Code Patch Reviewer