From: Sami Tolvanen <[email protected]>
For a prime-imported BO with no `gem_create_object` callback,
`__drm_gem_shmem_create()` allocates `kzalloc(sizeof(struct
drm_gem_shmem_object))`, which is smaller than the Rust `Object<T>`
wrapper. A later `container_of!()` from a `drm_gem_object` back to
`Object<T>` then runs past the allocation and corrupts the heap.
Provide a `gem_create_object` callback that allocates the full
`Object<T>` via `KBox::pin_init`. Move `obj` to the first field so the
returned pointer is at offset zero, satisfying the `kfree()` on the
init-failure path. This is now compile-checked with `const_assert!`.
Imported BOs have no userspace creation arguments, so extend
`DriverObject` with a `create_imported(dev, size)` constructor and
implement it for Tyr's `BoData`, Nova, and the in-tree KUnit driver.
Fixes: 80df573af9ef ("rust: drm: gem: shmem: Add DRM shmem helper abstraction")
Cc: <[email protected]> # no in tree users
Signed-off-by: Sami Tolvanen <[email protected]>
Signed-off-by: Janne Grunau <[email protected]>
---
drivers/gpu/drm/nova/gem.rs | 4 +++
drivers/gpu/drm/tyr/gem.rs | 4 +++
rust/kernel/drm/gem/mod.rs | 9 +++++++
rust/kernel/drm/gem/shmem.rs | 58 +++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index 2b6fe9dc0bfa..2e17422e76f5 100644
--- a/drivers/gpu/drm/nova/gem.rs
+++ b/drivers/gpu/drm/nova/gem.rs
@@ -27,6 +27,10 @@ impl gem::DriverObject for NovaObject {
fn new(_dev: &NovaDevice, _size: usize, _args: Self::Args) -> impl
PinInit<Self, Error> {
try_pin_init!(NovaObject {})
}
+
+ fn create_imported(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self,
Error> {
+ try_pin_init!(NovaObject {})
+ }
}
impl NovaObject {
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 3bf3787f5c3f..4b61bb04c845 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -48,6 +48,10 @@ impl gem::DriverObject for BoData {
fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl
PinInit<Self, Error> {
try_pin_init!(Self { flags: args.flags })
}
+
+ fn create_imported(_dev: &TyrDrmDevice, _size: usize) -> impl
PinInit<Self, Error> {
+ try_pin_init!(Self { flags: 0 })
+ }
}
/// Type alias for Tyr GEM buffer objects.
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index e1ebad77ebe2..e01b8311dd89 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -98,6 +98,15 @@ fn new(
args: Self::Args,
) -> impl PinInit<Self, Error>;
+ /// Create a new driver data object for a GEM object that is being imported
+ /// from another driver (e.g. via dma-buf).
+ ///
+ /// Imported objects are allocated by the importing helper, which only
+ /// provides the device and size. There is no driver-supplied
+ /// [`DriverObject::Args`], so drivers must initialize the per-object
+ /// state without those arguments.
+ fn create_imported(dev: &drm::Device<Self::Driver>, size: usize) -> impl
PinInit<Self, Error>;
+
/// Open a new handle to an existing object, associated with a File.
fn open(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) -> Result {
Ok(())
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index a687d46d170d..1df3554893cc 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -100,6 +100,12 @@ fn default() -> Self {
#[repr(C)]
#[pin_data]
pub struct Object<T: DriverObject> {
+ /// Embedded `drm_gem_shmem_object`.
+ ///
+ /// Must remain the first field so the allocation address coincides with
the embedded
+ /// `drm_gem_shmem_object`. The DRM shmem helper's
`__drm_gem_shmem_create()` error path calls
+ /// `kfree()` on the returned `drm_gem_object` pointer, which is only
sound when it points at
+ /// the start of the allocation.
#[pin]
obj: Opaque<bindings::drm_gem_shmem_object>,
/// Parent object that owns this object's DMA reservation object.
@@ -343,6 +349,49 @@ pub fn new(
Ok(obj)
}
+ /// Allocates the full [`Object<T>`] wrapper and zero-initializes the
embedded shmem object.
+ /// The helper's `__drm_gem_shmem_init` fills it in after we return.
+ extern "C" fn gem_create_object_callback(
+ raw_dev: *mut bindings::drm_device,
+ size: usize,
+ ) -> *mut bindings::drm_gem_object {
+ const_assert!(
+ core::mem::offset_of!(Self, obj) == 0,
+ "Object<T>::obj must be at offset 0 so drm_gem_shmem_helper's
kfree(obj) error path \
+ frees the wrapper allocation, not the middle of it"
+ );
+
+ // SAFETY: The DRM shmem helper only invokes this callback for a
registered DRM device of
+ // type `T::Driver`, which is `Self`'s driver per the `AllocImpl` impl
below.
+ let dev: &Device<T::Driver> = unsafe { Device::from_raw(raw_dev) };
+
+ let new: Pin<KBox<Self>> = match KBox::try_pin_init(
+ try_pin_init!(Self {
+ obj <- Opaque::init_zeroed(),
+ parent_resv_obj: None,
+ sgt_res: ManuallyDrop::new(SetOnce::new()),
+ sgt_lock <- new_mutex!(()),
+ inner <- T::create_imported(dev, size),
+ }),
+ GFP_KERNEL,
+ ) {
+ Ok(new) => new,
+ Err(e) => return e.to_ptr(),
+ };
+
+ // SAFETY: `new.as_raw()` is guaranteed to be valid by the
initialization above.
+ unsafe { (*new.as_raw()).funcs = &Self::VTABLE };
+
+ // SAFETY: We never move out of `self`. Ownership of the allocation is
transferred to the
+ // C side, which will run `__drm_gem_shmem_init()` next and later free
us via
+ // `free_callback` (or `kfree()` on the `__drm_gem_shmem_init()` error
path, matching the
+ // C kzalloc fallback contract).
+ let new = KBox::into_raw(unsafe { Pin::into_inner_unchecked(new) });
+
+ // SAFETY: `new` was just produced by `KBox::into_raw` and so is a
valid pointer to an
+ // initialized `Self`. `obj.base` is the embedded `drm_gem_object` the
caller expects.
+ unsafe { &raw mut (*(*new).obj.get()).base }
+ }
/// Creates and returns an owned reference to a virtual kernel memory
mapping for this object.
#[inline]
pub fn owned_vmap<const SIZE: usize>(&self) -> Result<VMapOwned<T, SIZE>> {
@@ -389,7 +438,7 @@ impl<T: DriverObject> driver::AllocImpl for Object<T> {
type Driver = T::Driver;
const ALLOC_OPS: driver::AllocOps = driver::AllocOps {
- gem_create_object: None,
+ gem_create_object: Some(Self::gem_create_object_callback),
prime_handle_to_fd: None,
prime_fd_to_handle: None,
gem_prime_import: None,
@@ -623,6 +672,13 @@ fn new(
) -> impl PinInit<Self, Error> {
try_pin_init!(KunitObject {})
}
+
+ fn create_imported(
+ _dev: &drm::Device<KunitDriver>,
+ _size: usize,
+ ) -> impl PinInit<Self, Error> {
+ try_pin_init!(KunitObject {})
+ }
}
#[vtable]
--
2.56.0
