On 10/15/26 7:00 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [High] gpu: nova-core: endianness bug when extracting u8/u32 arrays from
> native &[u64] stream
...
>> +++ 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(size_of::<u64>()))?;
>> + values.as_bytes().get(..count).ok_or(EINVAL)
>> + }
>
> [Severity: High]
> Does casting a native u64 slice to bytes extract the wrong payload on
> big-endian architectures?
Roman, or anyone who knows,
How do we update sashiko to make it stop reporting on this issue, which
we have decided is a non-issue (Nova is actually configured to only ever
be little-endian)?
thanks,
John Hubbard
>
> Because Cursor::take_u64s() reads host-native 64-bit integers, using
> .as_bytes() reverses the byte layout on big-endian hosts compared to
> little-endian.
>
> For example, if a 3-byte payload is stored inside a 64-bit word, a big-endian
> host will have the payload at the end of the word and zero-padding at the
> beginning, causing .get(..count) to extract the zeros instead of the actual
> data payload.
>
>> +
>> + /// Takes `count` 32-bit values. If `count` is not a multiple of 2
>> (`u32` values per `u64`),
>> + /// 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(size_of::<u64>() /
>> size_of::<u32>()))?;
>> + <[u32]>::ref_from_prefix_with_elems(values.as_bytes(), count)
>> + .map(|(elems, _)| elems)
>> + .map_err(|_| EINVAL)
>> + }
>
> [Severity: High]
> Can this result in swapped 32-bit words on big-endian systems?
>
> Similar to the byte array extraction above, passing the native memory
> representation through .as_bytes() to recreate a [u32] slice will swap
> the order of the 32-bit halves on big-endian architectures compared to
> little-endian ones.
>
> [ ... ]
>> + /// Decodes every pair into `schema` and returns the result of
>> [`Schema::finish`].
>> + pub(crate) fn decode<'s, S: Schema + Visit<'a>>(
>> + &self,
>> + schema: &'s mut S,
>> + ) -> Result<impl Init<S::Target, Error> + 's> {
>> + let mut cursor = Cursor::new(self.data);
>> + while !cursor.is_empty() {
>> + let op: Op = cursor.take_u64()?.into();
>
> [Severity: High]
> Does this misalign the operation bitfields on big-endian hosts?
>
> The 64-bit operation word is read as a host-native integer via take_u64().
> If the NVKV stream comes from a little-endian GSP, parsing it directly as a
> native u64 on a big-endian host will misalign the key, index, and value
> fields inside Op.
>
> Should this explicitly handle endianness (e.g., using little-endian read
> functions) instead of relying on native host integers?
>
