mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] drm/tyr: safely write GpuInfo to userspace
@ 2026-09-24  0:20 Deborah Brouwer
  2026-09-24 10:33 ` Alice Ryhl
  0 siblings, 1 reply; 2+ messages in thread
From: Deborah Brouwer @ 2026-09-24  0:20 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, David Airlie,
	Simona Vetter, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Tamir Duberstein, Alexandre Courbot,
	Onur Özkan
  Cc: dri-devel, rust-for-linux, linux-kernel, Deborah Brouwer

Tyr creates the struct drm_panthor_gpu_info from the Panthor UAPI
bindings. It initializes this struct by querying the gpu and then writes
the struct into userspace memory faithfully byte-by-byte.

Currently, the struct drm_panthor_gpu_info does not have any implicit
padding, but if implicit padding were added in the future Rust does not
guarantee that it would be initialized. Then when Tyr writes the struct
back to userspace it could expose uninitialized kernel memory.

Tyr implements the unsafe trait AsBytes for GpuInfo whereby the developer
guarantees that struct drm_panthor_gpu_info does not include implicit
padding, pointers, or interior mutability. This allows Tyr to safely copy
the struct into userspace memory. However, relying on the unsafe trait
AsBytes is fragile because if the Panthor UAPI changes, the SAFETY
guarantees may no longer be accurate.

Instead, use the macros provided by the zerocopy crate to safely derive
the traits IntoBytes and Immutable for struct drm_panthor_gpu_info. This
will cause a compile error if the Panthor UAPI changes in a way that the
traits IntoBytes or Immutable can no longer be implemented.

Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
---
 drivers/gpu/drm/tyr/file.rs |  2 +-
 drivers/gpu/drm/tyr/gpu.rs  | 12 ------------
 rust/Makefile               | 12 +++++++++---
 rust/bindgen_parameters     |  4 ++++
 rust/uapi/lib.rs            |  4 ++++
 5 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 9f60a90d4948..ceadde976502 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -54,7 +54,7 @@ pub(crate) fn dev_query(
                     )
                     .writer();
 
-                    writer.write(&reg_data.gpu_info)?;
+                    writer.write_slice(reg_data.gpu_info.as_bytes())?;
 
                     Ok(0)
                 }
diff --git a/drivers/gpu/drm/tyr/gpu.rs b/drivers/gpu/drm/tyr/gpu.rs
index e1b04a0c5159..fde5bd0f87a1 100644
--- a/drivers/gpu/drm/tyr/gpu.rs
+++ b/drivers/gpu/drm/tyr/gpu.rs
@@ -16,7 +16,6 @@
     },
     prelude::*,
     time::Delta,
-    transmute::AsBytes,
     uapi, //
 };
 
@@ -134,17 +133,6 @@ fn deref_mut(&mut self) -> &mut Self::Target {
     }
 }
 
-// SAFETY: `GpuInfo`'s invariant guarantees that it is the same type that is
-// already exposed to userspace by the C driver. This implies that it fulfills
-// the requirements for `AsBytes`.
-//
-// This means:
-//
-// - No implicit padding,
-// - No kernel pointers,
-// - No interior mutability.
-unsafe impl AsBytes for GpuInfo {}
-
 struct GpuModels {
     name: &'static str,
     arch_major: u32,
diff --git a/rust/Makefile b/rust/Makefile
index da1a7409d984..bf02a1efd6f3 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -365,8 +365,11 @@ rusttestlib-bindings: private rustc_target_flags = --extern ffi --extern pin_ini
 rusttestlib-bindings: $(src)/bindings/lib.rs rusttestlib-ffi rusttestlib-pin_init FORCE
 	+$(call if_changed,rustc_test_library)
 
-rusttestlib-uapi: private rustc_target_flags = --extern ffi --extern pin_init
-rusttestlib-uapi: $(src)/uapi/lib.rs rusttestlib-ffi rusttestlib-pin_init FORCE
+rusttestlib-uapi: private rustc_target_flags = --extern ffi --extern pin_init \
+	--extern zerocopy=$(objtree)/$(obj)/test/libzerocopy.rlib \
+	--extern zerocopy_derive=$(objtree)/$(obj)/test/$(libzerocopy_derive_name)
+rusttestlib-uapi: $(src)/uapi/lib.rs rusttestlib-ffi rusttestlib-pin_init \
+	rusttestlib-zerocopy rusttestlib-zerocopy_derive FORCE
 	+$(call if_changed,rustc_test_library)
 
 quiet_cmd_rustdoc_test = RUSTDOC T $<
@@ -782,11 +785,14 @@ $(obj)/bindings.o: $(src)/bindings/lib.rs \
     $(obj)/bindings/bindings_helpers_generated.rs FORCE
 	+$(call if_changed_rule,rustc_library)
 
-$(obj)/uapi.o: private rustc_target_flags = --extern ffi --extern pin_init
+$(obj)/uapi.o: private rustc_target_flags = --extern ffi --extern pin_init \
+	--extern zerocopy --extern zerocopy_derive
 $(obj)/uapi.o: private skip_gendwarfksyms = 1
 $(obj)/uapi.o: $(src)/uapi/lib.rs \
     $(obj)/ffi.o \
     $(obj)/pin_init.o \
+    $(obj)/zerocopy.o \
+    $(obj)/$(libzerocopy_derive_name) \
     $(obj)/uapi/uapi_generated.rs FORCE
 	+$(call if_changed_rule,rustc_library)
 
diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
index 8402b0c93545..65d913181eec 100644
--- a/rust/bindgen_parameters
+++ b/rust/bindgen_parameters
@@ -71,3 +71,7 @@
 # Structs should implement `Zeroable` when all of their fields do.
 --with-derive-custom-struct .*=MaybeZeroable
 --with-derive-custom-union .*=MaybeZeroable
+
+# `drm_panthor_gpu_info` is copied byte-for-byte to userspace.
+--with-derive-custom-struct '^drm_panthor_gpu_info$'=IntoBytes
+--with-derive-custom-struct '^drm_panthor_gpu_info$'=Immutable
diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
index 797ead5b5626..ddb35ffcd4b6 100644
--- a/rust/uapi/lib.rs
+++ b/rust/uapi/lib.rs
@@ -36,5 +36,9 @@
 type __kernel_ptrdiff_t = isize;
 
 use pin_init::MaybeZeroable;
+use zerocopy_derive::{
+    Immutable,
+    IntoBytes, //
+};
 
 include!(concat!(env!("OBJTREE"), "/rust/uapi/uapi_generated.rs"));

---
base-commit: 896ed083362758b33c49a1b5e5a3423c5814d87e
change-id: 20260923-b4-gpu_info_intobytes-c2d45f2935c8

Best regards,
-- 
Deborah Brouwer <deborah.brouwer@collabora.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/tyr: safely write GpuInfo to userspace
  2026-09-24  0:20 [PATCH] drm/tyr: safely write GpuInfo to userspace Deborah Brouwer
@ 2026-09-24 10:33 ` Alice Ryhl
  0 siblings, 0 replies; 2+ messages in thread
From: Alice Ryhl @ 2026-09-24 10:33 UTC (permalink / raw)
  To: Deborah Brouwer
  Cc: Danilo Krummrich, Daniel Almeida, David Airlie, Simona Vetter,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, dri-devel, rust-for-linux,
	linux-kernel

On Thu, Sep 24, 2026 at 2:21 AM Deborah Brouwer
<deborah.brouwer@collabora.com> wrote:
>
> Tyr creates the struct drm_panthor_gpu_info from the Panthor UAPI
> bindings. It initializes this struct by querying the gpu and then writes
> the struct into userspace memory faithfully byte-by-byte.
>
> Currently, the struct drm_panthor_gpu_info does not have any implicit
> padding, but if implicit padding were added in the future Rust does not
> guarantee that it would be initialized. Then when Tyr writes the struct
> back to userspace it could expose uninitialized kernel memory.
>
> Tyr implements the unsafe trait AsBytes for GpuInfo whereby the developer
> guarantees that struct drm_panthor_gpu_info does not include implicit
> padding, pointers, or interior mutability. This allows Tyr to safely copy
> the struct into userspace memory. However, relying on the unsafe trait
> AsBytes is fragile because if the Panthor UAPI changes, the SAFETY
> guarantees may no longer be accurate.
>
> Instead, use the macros provided by the zerocopy crate to safely derive
> the traits IntoBytes and Immutable for struct drm_panthor_gpu_info. This
> will cause a compile error if the Panthor UAPI changes in a way that the
> traits IntoBytes or Immutable can no longer be implemented.
>
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>

I checked it myself and per-type declarations are indeed needed. We
can't use most_traits for everything yet.

> diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
> index 8402b0c93545..65d913181eec 100644
> --- a/rust/bindgen_parameters
> +++ b/rust/bindgen_parameters
> @@ -71,3 +71,7 @@
>  # Structs should implement `Zeroable` when all of their fields do.
>  --with-derive-custom-struct .*=MaybeZeroable
>  --with-derive-custom-union .*=MaybeZeroable
> +
> +# `drm_panthor_gpu_info` is copied byte-for-byte to userspace.
> +--with-derive-custom-struct '^drm_panthor_gpu_info$'=IntoBytes
> +--with-derive-custom-struct '^drm_panthor_gpu_info$'=Immutable

For this case, I think we should just do:

--with-derive-custom-struct
'^drm_panthor_gpu_info$'=zerocopy_derive::most_traits

That will automatically give us these two traits.

Alice

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-24 10:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24  0:20 [PATCH] drm/tyr: safely write GpuInfo to userspace Deborah Brouwer
2026-09-24 10:33 ` Alice Ryhl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®