Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] nova-core: NVKV decode corrupts array layouts on big-endian hosts --
commit 561207bfbca10da1af8ade9ea863d375b4883ed5 Author: Eliot Courtney <[email protected]> gpu: nova-core: add NVKV decoder Add a decoder for NVKV. This is for receiving messages from GSP for GMCAPI calls. The NVKV format essentially encodes a sequence of function calls f(key, index, value). > diff --git a/drivers/gpu/nova-core/gsp/nvkv/decode.rs > b/drivers/gpu/nova-core/gsp/nvkv/decode.rs > new file mode 100644 > index 0000000000000..c4c24fe1108e9 > --- /dev/null > +++ b/drivers/gpu/nova-core/gsp/nvkv/decode.rs [ ... ] > + /// Takes `count` bytes. If `count` is not a multiple of 8 (`u64` size), > bytes are discarded up > + /// to the next multiple. > + fn take_u8s(&mut self, count: usize) -> Result<&'a [u8]> { > + let values = self.take_u64s(count.div_ceil(8))?; > + values.as_bytes().get(..count).ok_or(EINVAL) [Severity: High] Does Cursor::take_u8s() in drivers/gpu/nova-core/gsp/nvkv/decode.rs handle big-endian architectures correctly? Directly casting native u64 memory to u8 slices yields the bytes in reverse order within each 64-bit word on big-endian systems. This could cause the NVKV protocol to fail to correctly decode (and encode) `Array8` arrays, causing GPU initialization (GSP RPC) to fail completely or receive corrupted data. > + } > + > + /// Takes `count` 32-bit values. If `count` is not a multiple of 2 > (`u64` size), bytes are > + /// discarded up to the next multiple. > + fn take_u32s(&mut self, count: usize) -> Result<&'a [u32]> { > + let values = self.take_u64s(count.div_ceil(2))?; > + <[u32]>::ref_from_prefix_with_elems(values.as_bytes(), count) [Severity: High] Can Cursor::take_u32s() corrupt array layouts on big-endian hosts? Directly mapping native-endian u64 memory to u32 slices swaps the order of packed elements on big-endian systems (e.g., swapping the first and second 32-bit values within the 64-bit word). This defect prevents correctly decoding `Array32` and `Seq32` arrays on these platforms. > + .map(|(elems, _)| elems) > + .map_err(|_| EINVAL) > + } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
