public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Alexandre Courbot" <acourbot@nvidia.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Alistair Popple" <apopple@nvidia.com>
Cc: "John Hubbard" <jhubbard@nvidia.com>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>, "Zhi Wang" <zhiw@nvidia.com>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	<rust-for-linux@vger.kernel.org>,
	<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] gpu: nova-core: gsp: fix undefined behavior in command queue code
Date: Mon, 23 Mar 2026 16:44:16 +0000	[thread overview]
Message-ID: <DHAB9Z571FPB.25927BK1RGNGL@garyguo.net> (raw)
In-Reply-To: <20260323-cmdq-ub-fix-v2-1-77d1213c3f7f@nvidia.com>

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`

  reply	other threads:[~2026-03-23 16:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-23  5:40 [PATCH v2] gpu: nova-core: gsp: fix undefined behavior in command queue code Alexandre Courbot
2026-03-23 16:44 ` Gary Guo [this message]
2026-03-24 14:44   ` Alexandre Courbot
2026-03-24 14:45     ` Danilo Krummrich
2026-03-24 15:15     ` Gary Guo
2026-03-26  4:30       ` Alexandre Courbot
2026-03-26  4:51         ` Alexandre Courbot
2026-03-26 12:03           ` Gary Guo
2026-03-26 15:55             ` Alice Ryhl
2026-03-27  0:47             ` Alexandre Courbot
2026-03-28 13:09               ` Gary Guo
2026-03-28 14:53                 ` Alexandre Courbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DHAB9Z571FPB.25927BK1RGNGL@garyguo.net \
    --to=gary@garyguo.net \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ecourtney@nvidia.com \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=ttabi@nvidia.com \
    --cc=zhiw@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox