Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Firmware parsing uses native endianness for multi-byte fields, causing corruption on big-endian architectures. -- commit c535dd221b00c3e53bf9b453add3e6be0eec80f6 Author: Pedro Yudi Honda <[email protected]> drm/nova: use zerocopy in booter.rs This commit replaces transmute::FromBytes with zerocopy::FromBytes for firmware header parsing in firmware/booter.rs, and updates call sites accordingly. > diff --git a/drivers/gpu/nova-core/firmware/booter.rs > b/drivers/gpu/nova-core/firmware/booter.rs > index d9313ac361af3..0cda2c50fbbe9 100644 > --- a/drivers/gpu/nova-core/firmware/booter.rs > +++ b/drivers/gpu/nova-core/firmware/booter.rs [ ... ] > @@ -53,7 +52,7 @@ fn frombytes_at<S: FromBytes + Sized>(slice: &[u8], offset: > usize) -> Result<S> > /// Such firmwares have an application-specific payload that needs to be > patched with a given > /// signature. > #[repr(C)] > -#[derive(Debug, Clone)] > +#[derive(Debug, Clone, FromBytes)] > struct HsHeaderV2 { > /// Offset to the start of the signatures. > sig_prod_offset: u32, [Severity: High] This is a pre-existing issue, but do these hardware firmware structures need to use explicit little-endian types instead of native u32? Since frombytes_at constructs instances directly from byte slices, reading raw little-endian bytes into native u32 fields will result in incorrect offsets and sizes on big-endian architectures. This affects the HsHeaderV2 struct in drivers/gpu/nova-core/firmware/booter.rs. [ ... ] > @@ -145,6 +141,7 @@ fn signatures_iter(&'a self) -> Result<impl Iterator<Item > = BooterSignature<'a>> > > /// Signature parameters, as defined in the firmware. > #[repr(C)] > +#[derive(FromBytes)] > struct HsSignatureParams { > /// Fuse version to use. > fuse_ver: u32, [Severity: High] This is a pre-existing issue, but does the HsSignatureParams struct in drivers/gpu/nova-core/firmware/booter.rs also suffer from the same endianness problem by using native u32 for firmware fields? [ ... ] > @@ -171,14 +165,14 @@ fn new(hs_fw: &HsFirmwareV2<'_>) -> Result<Self> { > /// Header for code and data load offsets. > #[repr(C)] > -#[derive(Debug, Clone)] > +#[derive(Debug, Clone, FromBytes)] > struct HsLoadHeaderV2 { > // Offset at which the code starts. > os_code_offset: u32, [Severity: High] This is a pre-existing issue, but does the HsLoadHeaderV2 struct in drivers/gpu/nova-core/firmware/booter.rs need explicit endianness types here as well? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
