Push the computation of the falcon data offset into a helper function. The subtraction to create the offset should be checked, and by doing this the check can be folded into the existing check in `falcon_data_ptr`.
Reviewed-by: John Hubbard <[email protected]> Signed-off-by: Eliot Courtney <[email protected]> --- drivers/gpu/nova-core/vbios.rs | 48 +++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs index cadc6dcffefb..ca101b2b6095 100644 --- a/drivers/gpu/nova-core/vbios.rs +++ b/drivers/gpu/nova-core/vbios.rs @@ -846,33 +846,29 @@ fn get_bit_token(&self, token_id: u8) -> Result<BitToken> { BitToken::from_id(self, token_id) } - /// Find the Falcon data pointer structure in the [`PciAtBiosImage`]. + /// Find the Falcon data offset from the start of the FWSEC region. /// - /// This is just a 4 byte structure that contains a pointer to the Falcon data in the FWSEC - /// image. - fn falcon_data_ptr(&self) -> Result<u32> { + /// The BIT table contains a 4-byte pointer to the Falcon data. Testing shows this pointer + /// treats the PCI-AT and FWSEC images as logically contiguous even when an EFI image sits in + /// between them, so subtract the PCI-AT image size here to convert it to a FWSEC-relative + /// offset. + fn falcon_data_offset(&self) -> Result<usize> { let token = self.get_bit_token(BIT_TOKEN_ID_FALCON_DATA)?; - - // Make sure we don't go out of bounds - if usize::from(token.data_offset) + 4 > self.base.data.len() { - return Err(EINVAL); - } - - // read the 4 bytes at the offset specified in the token let offset = usize::from(token.data_offset); - let bytes: [u8; 4] = self.base.data[offset..offset + 4].try_into().map_err(|_| { - dev_err!(self.base.dev, "Failed to convert data slice to array\n"); - EINVAL - })?; - let data_ptr = u32::from_le_bytes(bytes); + // Read the 4-byte falcon data pointer at the offset specified in the token. + let data = &self.base.data; + let (ptr, _) = data + .get(offset..) + .and_then(u32::from_bytes_copy_prefix) + .ok_or(EINVAL)?; - if (usize::from_safe_cast(data_ptr)) < self.base.data.len() { - dev_err!(self.base.dev, "Falcon data pointer out of bounds\n"); - return Err(EINVAL); - } - - Ok(data_ptr) + usize::from_safe_cast(ptr) + .checked_sub(data.len()) + .ok_or(EINVAL) + .inspect_err(|_| { + dev_err!(self.base.dev, "Falcon data pointer out of bounds\n"); + }) } } @@ -989,15 +985,9 @@ fn setup_falcon_data( pci_at_image: &PciAtBiosImage, first_fwsec: &FwSecBiosBuilder, ) -> Result { - let mut offset = usize::from_safe_cast(pci_at_image.falcon_data_ptr()?); + let mut offset = pci_at_image.falcon_data_offset()?; let mut pmu_in_first_fwsec = false; - // The falcon data pointer assumes that the PciAt and FWSEC images - // are contiguous in memory. However, testing shows the EFI image sits in - // between them. So calculate the offset from the end of the PciAt image - // rather than the start of it. Compensate. - offset -= pci_at_image.base.data.len(); - // The offset is now from the start of the first Fwsec image, however // the offset points to a location in the second Fwsec image. Since // the fwsec images are contiguous, subtract the length of the first Fwsec -- 2.54.0
