On Sat Nov 15, 2025 at 8:30 AM JST, Timur Tabi wrote:
<snip>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index abf423560ff4..860d6fb3f516 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -19,6 +19,8 @@
> driver::Bar0,
> firmware::{
> fwsec::Bcrt30Rsa3kSignature,
> + FalconUCodeDesc,
> + FalconUCodeDescV2,
> FalconUCodeDescV3, //
> },
> num::FromSafeCast,
> @@ -1004,19 +1006,10 @@ fn build(self) -> Result<FwSecBiosImage> {
>
> impl FwSecBiosImage {
> /// Get the FwSec header ([`FalconUCodeDescV3`]).
> - pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3> {
> + 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;
>
> - // Make sure the offset is within the data bounds.
> - if falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>() >
> self.base.data.len() {
> - dev_err!(
> - self.base.dev,
> - "fwsec-frts header not contained within BIOS bounds\n"
> - );
> - return Err(ERANGE);
> - }
> -
> // 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()
> @@ -1024,33 +1017,60 @@ pub(crate) fn header(&self) ->
> Result<&FalconUCodeDescV3> {
> let hdr = u32::from_le_bytes(hdr_bytes);
> let ver = (hdr & 0xff00) >> 8;
>
> - if ver != 3 {
> - dev_err!(self.base.dev, "invalid fwsec firmware version:
> {:?}\n", ver);
> - return Err(EINVAL);
> + let hdr_size = match ver {
> + 2 => core::mem::size_of::<FalconUCodeDescV2>(),
> + 3 => core::mem::size_of::<FalconUCodeDescV3>(),
> + _ => {
> + dev_err!(self.base.dev, "invalid fwsec firmware version:
> {:?}\n", ver);
> + return Err(EINVAL);
> + }
> + };
> + // Make sure the offset is within the data bounds
> + if falcon_ucode_offset + hdr_size > self.base.data.len() {
> + dev_err!(
> + self.base.dev,
> + "fwsec-frts header not contained within BIOS bounds\n"
> + );
> + return Err(ERANGE);
> }
>
> - // Return a reference to the FalconUCodeDescV3 structure.
> - //
> - // SAFETY: We have checked that `falcon_ucode_offset +
> size_of::<FalconUCodeDescV3>` is
> - // within the bounds of `data`. Also, this data vector is from ROM,
> and the `data` field
> - // in `BiosImageBase` is immutable after construction.
> - Ok(unsafe {
> + let v2 = unsafe {
> + &*(self
> + .base
> + .data
> + .as_ptr()
> + .add(falcon_ucode_offset)
> + .cast::<FalconUCodeDescV2>())
> + };
> +
> + let v3 = unsafe {
> &*(self
> .base
> .data
> .as_ptr()
> .add(falcon_ucode_offset)
> .cast::<FalconUCodeDescV3>())
> - })
> + };
Mmm, we are creating references to both versions of the header, one of
which is going to be invalid.
Let's do the casting in the match arm below:
match ver {
2 => {
let v2 = unsafe {
...
};
Ok(FalconUCodeDesc::V2(v2.clone()))
}
3 => {
let v3 = unsafe {
...
};
Ok(FalconUCodeDesc::V3(v3.clone()))
}
}
