[PATCH v2] gpu: nova-core: gsp: fix undefined behavior in command queue code

[PATCH v2] gpu: nova-core: gsp: fix undefined behavior in command queue code

[Alexandre Courbot]

`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 <dakr@kernel.org>
Closes: https://lore.kernel.org/all/DH47AVPEKN06.3BERUSJIB4M1R@kernel.org/
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
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/20260319-cmdq-ub-fix-v1-1-0f9f6e8f3ce3@nvidia.com
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 133 ++++++++++++++++++++++++--------------
 1 file changed, 84 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index d36a62ba1c60..4d73a299f3da 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -251,38 +251,53 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
     /// As the message queue is a circular buffer, the region may be discontiguous in memory. In
     /// that case the second slice will have a non-zero length.
     fn driver_write_area(&mut self) -> (&mut [[u8; GSP_PAGE_SIZE]], &mut [[u8; GSP_PAGE_SIZE]]) {
-        let tx = self.cpu_write_ptr() as usize;
-        let rx = self.gsp_read_ptr() as usize;
+        let tx = num::u32_as_usize(self.cpu_write_ptr());
+        let rx = num::u32_as_usize(self.gsp_read_ptr());
+
+        // Pointer to the start of the CPU message queue.
+        //
+        // SAFETY:
+        // - `self.0` contains exactly one element.
+        // - `cpuq.msgq.data[0]` is within the bounds of that element.
+        let data = unsafe { &raw mut (*self.0.start_ptr_mut()).cpuq.msgq.data[0] };
+
+        // The two slices we return:
+        // - The tail slice starts at `tx` and ends either at `rx - 1` (if the area doesn't
+        //   wrap) or the end of the queue.
+        // - The wrap slice starts at `0` and is either empty (if the area doesn't wrap) or
+        //   ends at `rx - 1`.
+        // One empty slot is always left before `rx` as a sentinel.
+        //
+        // INVARIANTS:
+        // Per the invariants of `gsp_read_ptr` and `cpu_write_ptr`, `tx` and `rx` are in the range
+        // `0..MSGQ_NUM_PAGES`, so:
+        // - `tx <= tail_end <= MSGQ_NUM_PAGES`.
+        // - `wrap_end < MSGQ_NUM_PAGES`.
+        let (tail_end, wrap_end) = if rx == 0 {
+            // Leave an empty slot at end of the buffer.
+            (num::u32_as_usize(MSGQ_NUM_PAGES) - 1, 0)
+        } else if rx <= tx {
+            // Discontiguous, leave an empty slot before `rx`.
+            (num::u32_as_usize(MSGQ_NUM_PAGES), rx - 1)
+        } else {
+            // Contiguous, leave an empty slot before `rx`.
+            (rx - 1, 0)
+        };
 
         // SAFETY:
-        // - The `CoherentAllocation` contains exactly one object.
-        // - 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 = &mut unsafe { self.0.as_slice_mut(0, 1) }.unwrap()[0];
-        // PANIC: per the invariant of `cpu_write_ptr`, `tx` is `< MSGQ_NUM_PAGES`.
-        let (before_tx, after_tx) = gsp_mem.cpuq.msgq.data.split_at_mut(tx);
-
-        // The area starting at `tx` and ending at `rx - 2` modulo MSGQ_NUM_PAGES, inclusive,
-        // belongs to the driver for writing.
-
-        if rx == 0 {
-            // Since `rx` is zero, leave an empty slot at end of the buffer.
-            let last = after_tx.len() - 1;
-            (&mut after_tx[..last], &mut [])
-        } else if rx <= tx {
-            // The area is discontiguous and we leave an empty slot before `rx`.
-            // PANIC:
-            // - The index `rx - 1` is non-negative because `rx != 0` in this branch.
-            // - The index does not exceed `before_tx.len()` (which equals `tx`) because
-            //   `rx <= tx` in this branch.
-            (after_tx, &mut before_tx[..(rx - 1)])
-        } else {
-            // The area is contiguous and we leave an empty slot before `rx`.
-            // PANIC:
-            // - The index `rx - tx - 1` is non-negative because `rx > tx` in this branch.
-            // - The index does not exceed `after_tx.len()` (which is `MSGQ_NUM_PAGES - tx`)
-            //   because `rx < MSGQ_NUM_PAGES` by the `gsp_read_ptr` invariant.
-            (&mut after_tx[..(rx - tx - 1)], &mut [])
+        // - `data` points to an array of `MSGQ_NUM_PAGES` elements.
+        // - The area starting at `tx` and ending at `rx - 2` modulo `MSGQ_NUM_PAGES`,
+        //   inclusive, belongs to the driver for writing and is not accessed concurrently by
+        //   the GSP.
+        // - `data.add(tx)` advances `tx` elements, and `tail_end - tx` further elements are
+        //   accessed. Per the invariant above, `tail_end <= MSGQ_NUM_PAGES`, so
+        //   `tx + (tail_end - tx)` = `tail_end` does not exceed the array bounds.
+        // - Per the invariant above, `wrap_end < MSGQ_NUM_PAGES`.
+        unsafe {
+            (
+                core::slice::from_raw_parts_mut(data.add(tx), tail_end - tx),
+                core::slice::from_raw_parts_mut(data, wrap_end),
+            )
         }
     }
 
@@ -305,27 +320,47 @@ fn driver_write_area_size(&self) -> usize {
     /// As the message queue is a circular buffer, the region may be discontiguous in memory. In
     /// that case the second slice will have a non-zero length.
     fn driver_read_area(&self) -> (&[[u8; GSP_PAGE_SIZE]], &[[u8; GSP_PAGE_SIZE]]) {
-        let tx = self.gsp_write_ptr() as usize;
-        let rx = self.cpu_read_ptr() as usize;
+        let tx = num::u32_as_usize(self.gsp_write_ptr());
+        let rx = num::u32_as_usize(self.cpu_read_ptr());
+
+        // Pointer to the start of the GSP message queue.
+        //
+        // SAFETY:
+        // - `self.0` contains exactly one element.
+        // - `gspq.msgq.data[0]` is within the bounds of that element.
+        let data = unsafe { &raw const (*self.0.start_ptr()).gspq.msgq.data[0] };
+
+        // The two slices we return:
+        // - The tail slice starts at `rx` and ends either at `tx` (if the area doesn't wrap) or
+        //   the end of the queue.
+        // - The wrap slice starts at `0` and is either empty (if the area doesn't wrap) or ends at
+        //   `tx`.
+        //
+        // INVARIANTS:
+        // Per the invariants of `cpu_read_ptr` and `gsp_write_ptr`, `tx` and `rx` are in the range
+        // `0..MSGQ_NUM_PAGES`, so:
+        // - `rx <= tail_end <= MSGQ_NUM_PAGES`.
+        // - `wrap_end < MSGQ_NUM_PAGES`.
+        let (tail_end, wrap_end) = if rx <= tx {
+            (tx, 0)
+        } else {
+            (num::u32_as_usize(MSGQ_NUM_PAGES), tx)
+        };
 
         // SAFETY:
-        // - The `CoherentAllocation` contains exactly one object.
-        // - 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_slice(0, 1) }.unwrap()[0];
-        let data = &gsp_mem.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 {
-            // The area is contiguous.
-            (&data[rx..tx], &[])
-        } else {
-            // The area is discontiguous.
-            (&data[rx..], &data[..tx])
+        // - `data` points to an array of `MSGQ_NUM_PAGES` elements.
+        // - The area starting at `rx` and ending at `tx - 1` modulo `MSGQ_NUM_PAGES`,
+        //   inclusive, belongs to the driver for reading and is not accessed concurrently by
+        //   the GSP.
+        // - `data.add(rx)` advances `rx` elements, and `tail_end - rx` further elements are
+        //   accessed. Per the invariant above, `tail_end <= MSGQ_NUM_PAGES`, so `rx + (tail_end -
+        //   rx)` = `tail_end` does not exceed the array bounds.
+        // - Per the invariant above, `wrap_end < MSGQ_NUM_PAGES`.
+        unsafe {
+            (
+                core::slice::from_raw_parts(data.add(rx), tail_end - rx),
+                core::slice::from_raw_parts(data, wrap_end),
+            )
         }
     }
 

---
base-commit: a19457958c3018783881c4416f272cd594f13049
change-id: 20260319-cmdq-ub-fix-d57b09a745b9

Best regards,
-- 
Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH v2] gpu: nova-core: gsp: fix undefined behavior in command queue code

[Gary Guo]

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 <dakr@kernel.org>
> Closes: https://lore.kernel.org/all/DH47AVPEKN06.3BERUSJIB4M1R@kernel.org/
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> 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/20260319-cmdq-ub-fix-v1-1-0f9f6e8f3ce3@nvidia.com

Here's the diff that fixes the issue using I/O projection
https://lore.kernel.org/rust-for-linux/20260323153807.1360705-1-gary@kernel.org/

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`

Re: [PATCH v2] gpu: nova-core: gsp: fix undefined behavior in command queue code

[Alexandre Courbot]

On Tue Mar 24, 2026 at 1:44 AM JST, Gary Guo wrote:
> 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 <dakr@kernel.org>
>> Closes: https://lore.kernel.org/all/DH47AVPEKN06.3BERUSJIB4M1R@kernel.org/
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> 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/20260319-cmdq-ub-fix-v1-1-0f9f6e8f3ce3@nvidia.com
>
> Here's the diff that fixes the issue using I/O projection
> https://lore.kernel.org/rust-for-linux/20260323153807.1360705-1-gary@kernel.org/

Should we apply or drop this patch meanwhile? I/O projections are still
undergoing review, but I'm fine with dropping it if Danilo thinks we can
live a bit longer with that UB. It's not like the driver is actively
doing anything useful yet anyway.

Re: [PATCH v2] gpu: nova-core: gsp: fix undefined behavior in command queue code

[Danilo Krummrich]

On Tue Mar 24, 2026 at 3:44 PM CET, Alexandre Courbot wrote:
> Should we apply or drop this patch meanwhile? I/O projections are still
> undergoing review, but I'm fine with dropping it if Danilo thinks we can
> live a bit longer with that UB. It's not like the driver is actively
> doing anything useful yet anyway.

We should pick this up regardless, I/O projections won't we usable for drm-rust
this cycle.

Thanks,
Danilo

Re: [PATCH v2] gpu: nova-core: gsp: fix undefined behavior in command queue code

[Gary Guo]

On Tue Mar 24, 2026 at 2:44 PM GMT, Alexandre Courbot wrote:
> On Tue Mar 24, 2026 at 1:44 AM JST, Gary Guo wrote:
>> 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 <dakr@kernel.org>
>>> Closes: https://lore.kernel.org/all/DH47AVPEKN06.3BERUSJIB4M1R@kernel.org/
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> ---
>>> 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/20260319-cmdq-ub-fix-v1-1-0f9f6e8f3ce3@nvidia.com
>>
>> Here's the diff that fixes the issue using I/O projection
>> https://lore.kernel.org/rust-for-linux/20260323153807.1360705-1-gary@kernel.org/
>
> Should we apply or drop this patch meanwhile? I/O projections are still
> undergoing review, but I'm fine with dropping it if Danilo thinks we can
> live a bit longer with that UB. It's not like the driver is actively
> doing anything useful yet anyway.

I want to avoid big changes back and forth. We could use raw pointer projection
today, which could be fairly easy to convert to I/O projection:

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 191b648e2ede..4cdbeed04294 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -23,6 +23,7 @@
     },
     new_mutex,
     prelude::*,
+    ptr::project as ptr_project,
     sync::{
         aref::ARef,
         Mutex, //
@@ -306,24 +307,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 = ptr_project!(self.0.as_ptr(), .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], &[])
+            (ptr_project!(data, [rx..tx]), ptr_project!(data, [..0]))
         } else {
             // The area is discontiguous.
-            (&data[rx..], &data[..tx])
-        }
+            (ptr_project!(data, [rx..]), ptr_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 }, unsafe { &*second })
     }
 
     /// Allocates a region on the command queue that is large enough to send a command of `size`

Claude review: gpu: nova-core: gsp: fix undefined behavior in command queue code

[Claude Code Review Bot]

Overall Series Review

Subject: gpu: nova-core: gsp: fix undefined behavior in command queue code
Author: Alexandre Courbot <acourbot@nvidia.com>
Patches: 5
Reviewed: 2026-03-25T08:06:34.022273

---

This is a single-patch fix for undefined behavior in the nova-core GSP command queue code. The old code called `as_slice_mut()`/`as_slice()` on the `CoherentAllocation`, which temporarily creates a Rust reference spanning the entire `GspMem` — including regions owned by the GSP that can change concurrently. Creating a `&`/`&mut` reference to memory that may be concurrently modified is UB in Rust's aliasing model, even if the returned sub-slices only cover driver-owned regions.

The fix replaces this with raw pointer arithmetic via `&raw mut`/`&raw const` and `core::slice::from_raw_parts[_mut]`, constructing slices that cover only the driver-owned portions. The approach is correct and the circular buffer logic is preserved exactly. The safety comments are thorough and reference the relevant invariants.

**Verdict: The patch looks correct and ready to merge.** One minor observation below, but nothing blocking.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: fix undefined behavior in command queue code

[Claude Code Review Bot]

Patch Review

**Correctness of logic transformation:**

The three branches in `driver_write_area` compute `(tail_end, wrap_end)` identically to the old split-based logic:

- `rx == 0` → `(MSGQ_NUM_PAGES - 1, 0)` — leaves sentinel at buffer end, no wrap. Matches old `(&mut after_tx[..last], &mut [])`.
- `rx <= tx` (and `rx != 0`) → `(MSGQ_NUM_PAGES, rx - 1)` — full tail, wrap up to sentinel before rx. Matches old `(after_tx, &mut before_tx[..(rx - 1)])`.
- `rx > tx` → `(rx - 1, 0)` — contiguous region `tx..rx-1`, no wrap. Matches old `(&mut after_tx[..(rx - tx - 1)], &mut [])`.

Similarly, `driver_read_area` is equivalent:
- `rx <= tx` → `(tx, 0)` — contiguous read region. Matches old `(&data[rx..tx], &[])`.
- `rx > tx` → `(MSGQ_NUM_PAGES, tx)` — discontiguous. Matches old `(&data[rx..], &data[..tx])`.

Both are correct.

**Raw pointer safety:**

The use of `&raw mut (*self.0.start_ptr_mut()).cpuq.msgq.data[0]` (line ~201 in the diff) is the right pattern — it obtains a raw pointer to the first element of the data array without ever creating an intermediate reference to the full `GspMem`. The `start_ptr_mut()` returns a `*mut GspMem`, and the field projection + indexing stays in raw-pointer land. Same for the `&raw const` variant in `driver_read_area`.

**Safety comments are adequate.** They reference the invariants of the pointer accessors and explain why the computed indices stay within bounds.

**Minor observation (non-blocking):**

In `driver_write_area`, the INVARIANTS comment states:
```
// - `wrap_end < MSGQ_NUM_PAGES`.
```

This is correct for the `rx == 0` case (wrap_end = 0) and the `rx > tx` case (wrap_end = 0), but for the `rx <= tx` case, `wrap_end = rx - 1`. Since `rx` ranges from 1 to `tx` (and `tx < MSGQ_NUM_PAGES` per `cpu_write_ptr` invariant), `wrap_end` can be at most `MSGQ_NUM_PAGES - 2`, so the bound holds. The reasoning is sound but could be slightly more explicit about *why* `rx - 1 < MSGQ_NUM_PAGES` — though arguably that's obvious given `rx < MSGQ_NUM_PAGES` from the pointer invariant.

**No bugs or correctness issues found.**

---
Generated by Claude Code Patch Reviewer