On Mon Mar 23, 2026 at 5:40 AM GMT, Alexandre Courbot wrote:
> `driver_read_area` and `driver_write_area` are internal methods that
> return slices containing the area of the command queue buffer that the
> driver has exclusive read or write access, respectively.
>
> While their returned value is correct and safe to use, internally they
> temporarily create a reference to the whole command-buffer slice,
> including GSP-owned regions. These regions can change without notice,
> and thus creating a slice to them is undefined behavior.
>
> Fix this by replacing the slice logic with pointer arithmetic and
> creating slices to valid regions only. It adds unsafe code, but should
> be mostly replaced by `IoView` and `IoSlice` once they land.
>
> Fixes: 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command queue bindings and
> handling")
> Reported-by: Danilo Krummrich <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Alexandre Courbot <[email protected]>
> ---
> I didn't apply Eliot's Reviewed-by because the code has changed
> drastically. The logic should remain identical though.
> ---
> Changes in v2:
> - Use `u32_as_usize` consistently.
> - Reduce the number of `unsafe` blocks by computing the end offset of
> the returned slices and creating them at the end, in one step.
> - Take advantage of the fact that both slices have the same start index
> regardless of the branch chosen.
> - Improve safety comments.
> - Link to v1:
> https://patch.msgid.link/[email protected]
Here's the diff that fixes the issue using I/O projection
https://lore.kernel.org/rust-for-linux/[email protected]/
Best,
Gary
-- >8 --
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs
b/drivers/gpu/nova-core/gsp/cmdq.rs
index 191b648e2ede..c759a81b28df 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -306,24 +306,25 @@ fn driver_write_area_size(&self) -> usize {
let tx = self.gsp_write_ptr() as usize;
let rx = self.cpu_read_ptr() as usize;
- // SAFETY:
- // - We will only access the driver-owned part of the shared memory.
- // - Per the safety statement of the function, no concurrent access
will be performed.
- let gsp_mem = unsafe { &*self.0.as_ptr() };
- let data = &gsp_mem.gspq.msgq.data;
+ let data = io_project!(self.0, .gspq.msgq.data);
// The area starting at `rx` and ending at `tx - 1` modulo
MSGQ_NUM_PAGES, inclusive,
// belongs to the driver for reading.
// PANIC:
// - per the invariant of `cpu_read_ptr`, `rx < MSGQ_NUM_PAGES`
// - per the invariant of `gsp_write_ptr`, `tx < MSGQ_NUM_PAGES`
- if rx <= tx {
+ let (first, second) = if rx <= tx {
// The area is contiguous.
- (&data[rx..tx], &[])
+ (io_project!(data, [rx..tx]), io_project!(data, [..0]))
} else {
// The area is discontiguous.
- (&data[rx..], &data[..tx])
- }
+ (io_project!(data, [rx..]), io_project!(data, [..tx]))
+ };
+
+ // SAFETY:
+ // - We will only access the driver-owned part of the shared memory.
+ // - Per the safety statement of the function, no concurrent access
will be performed.
+ (unsafe { first.as_ref() }, unsafe { second.as_ref() })
}
/// Allocates a region on the command queue that is large enough to send a
command of `size`
