Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Deriving `FromBytes` on `Region<const SIZE: usize>` allows safe code to bypass the struct's documented `SIZE` invariant. - [Low] Removal of trailing `//` on the vertical import block closing brace violates kernel Rust formatting rules. --
commit 0616e76ffd1d8525b38835ead9f29afeae45c2ed Author: Gary Guo <[email protected]> Subject: Add an io_project!() macro allows projection from Io to a subview This commit introduces an io_project!() macro that allows projection from Io to a subview using compile-time checks. It also adds a try_cast() function to perform size and alignment checks at runtime when type-casting is needed. > diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs > index 470ee2ed9849..c7a9952af995 100644 > --- a/rust/kernel/io.rs > +++ b/rust/kernel/io.rs > @@ -14,7 +14,7 @@ > ptr::{ > Alignment, > KnownSize, // > - }, // > + }, [Severity: Low] This isn't a bug, but does removing the trailing // on the closing brace violate the kernel Rust vertical import formatting rules? The coding guidelines require this marker to prevent rustfmt from collapsing the multi-line imports. > }; > > pub mod mem; [ ... ] > @@ -44,12 +44,12 @@ > /// This type can be used when an I/O region without known type information > has a compile-time known > /// minimum size (and a runtime known actual size). > /// > -/// This must be 4-byte aligned. > -/// > /// # Invariants > /// > -/// Size of the region is at least as large as the `SIZE` generic parameter. > +/// - Size of the region is at least as large as the `SIZE` generic > parameter. > +/// - Size of the region is multiple of 4. > #[repr(C, align(4))] > +#[derive(FromBytes)] > pub struct Region<const SIZE: usize = 0> { > inner: [u8], > } [Severity: High] Does deriving FromBytes on Region allow safe code to bypass the documented SIZE invariant? By implementing zerocopy::FromBytes, safe code could use ref_from() to cast any 4-byte aligned byte slice into a &Region. Since zerocopy only checks that the slice length matches the dynamically computed layout size, which for inner: [u8] is just the length of the slice, it appears safe code could cast a 4-byte slice into a &Region<1024>. Could this lead to out-of-bounds reads or writes if unsafe code subsequently relies on the SIZE parameter for bounds checking? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
