Re: [PATCH] gpu: nova-core: gsp: fix UB in DmaGspMem pointer accessors

Eliot Courtney
2026-03-10T03:57:02Z

On Tue Mar 10, 2026 at 11:01 AM JST, Gary Guo wrote:
>> + pub(in crate::gsp) fn advance_cpu_read_ptr(qs: &CoherentAllocation<GspMem>, count: u32) {
>> + let rptr = cpu_read_ptr(qs).wrapping_add(count) % MSGQ_NUM_PAGES;
>> +
>> + // Ensure read pointer is properly ordered.
>> + fence(Ordering::SeqCst);
>> +
>> + // PANIC: A `dma::CoherentAllocation` always contains at least one element.
>> + || -> Result {
>> + dma_write!(qs, [0]?.cpuq.rx.0.readPtr, rptr);
>> + Ok(())
>> + }()
>> + .unwrap()
>> + }
>> +
>> + pub(in crate::gsp) fn cpu_write_ptr(qs: &CoherentAllocation<GspMem>) -> u32 {
>> + // PANIC: A `dma::CoherentAllocation` always contains at least one element.
>> + || -> Result<u32> { Ok(dma_read!(qs, [0]?.cpuq.tx.0.writePtr) % MSGQ_NUM_PAGES) }().unwrap()
>> + }
>> +
>> + pub(in crate::gsp) fn advance_cpu_write_ptr(qs: &CoherentAllocation<GspMem>, count: u32) {
>> + let wptr = cpu_write_ptr(qs).wrapping_add(count) & MSGQ_NUM_PAGES;
>
> Not really related to your change, but this `&` probably require a comment, as
> it has different behaviour compared to `%` given the `MSGQ_NUM_PAGES` is not
> power of two. I suppose this is actually intended so there's a way to
> distinguish between empty and full ring buffer?
>
> Best,
> Gary

This is actually incorrect and I have fixed it here[1]. I think it
should be merged in drm-rust-next now.

[1]: https://lore.kernel.org/all/20260129-nova-core-cmdq1-v3-0-2ede85493a27@nvidia.com/