From fde578c86281f27b182680c7642836a0dbbd0be7 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 31 Jul 2025 17:48:06 +0200 Subject: rust: alloc: replace aligned_size() with Kmalloc::aligned_layout() aligned_size() dates back to when Rust did support kmalloc() only, but is now used in ReallocFunc::call() and hence for all allocators. However, the additional padding applied by aligned_size() is only required by the kmalloc() allocator backend. Hence, replace aligned_size() with Kmalloc::aligned_layout() and use it for the affected allocators, i.e. kmalloc() and kvmalloc(), only. While at it, make Kmalloc::aligned_layout() public, such that Rust abstractions, which have to call subsystem specific kmalloc() based allocation primitives directly, can make use of it. Fixes: 8a799831fc63 ("rust: alloc: implement `ReallocFunc`") Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250731154919.4132-2-dakr@kernel.org [ Remove `const` from Kmalloc::aligned_layout(). - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/allocator.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs index aa2dfa9dca4c..2692cf90c948 100644 --- a/rust/kernel/alloc/allocator.rs +++ b/rust/kernel/alloc/allocator.rs @@ -43,17 +43,6 @@ pub struct Vmalloc; /// For more details see [self]. pub struct KVmalloc; -/// Returns a proper size to alloc a new object aligned to `new_layout`'s alignment. -fn aligned_size(new_layout: Layout) -> usize { - // Customized layouts from `Layout::from_size_align()` can have size < align, so pad first. - let layout = new_layout.pad_to_align(); - - // Note that `layout.size()` (after padding) is guaranteed to be a multiple of `layout.align()` - // which together with the slab guarantees means the `krealloc` will return a properly aligned - // object (see comments in `kmalloc()` for more information). - layout.size() -} - /// # Invariants /// /// One of the following: `krealloc`, `vrealloc`, `kvrealloc`. @@ -88,7 +77,7 @@ impl ReallocFunc { old_layout: Layout, flags: Flags, ) -> Result, AllocError> { - let size = aligned_size(layout); + let size = layout.size(); let ptr = match ptr { Some(ptr) => { if old_layout.size() == 0 { @@ -123,6 +112,17 @@ impl ReallocFunc { } } +impl Kmalloc { + /// Returns a [`Layout`] that makes [`Kmalloc`] fulfill the requested size and alignment of + /// `layout`. + pub fn aligned_layout(layout: Layout) -> Layout { + // Note that `layout.size()` (after padding) is guaranteed to be a multiple of + // `layout.align()` which together with the slab guarantees means that `Kmalloc` will return + // a properly aligned object (see comments in `kmalloc()` for more information). + layout.pad_to_align() + } +} + // SAFETY: `realloc` delegates to `ReallocFunc::call`, which guarantees that // - memory remains valid until it is explicitly freed, // - passing a pointer to a valid memory allocation is OK, @@ -135,6 +135,8 @@ unsafe impl Allocator for Kmalloc { old_layout: Layout, flags: Flags, ) -> Result, AllocError> { + let layout = Kmalloc::aligned_layout(layout); + // SAFETY: `ReallocFunc::call` has the same safety requirements as `Allocator::realloc`. unsafe { ReallocFunc::KREALLOC.call(ptr, layout, old_layout, flags) } } @@ -176,6 +178,10 @@ unsafe impl Allocator for KVmalloc { old_layout: Layout, flags: Flags, ) -> Result, AllocError> { + // `KVmalloc` may use the `Kmalloc` backend, hence we have to enforce a `Kmalloc` + // compatible layout. + let layout = Kmalloc::aligned_layout(layout); + // TODO: Support alignments larger than PAGE_SIZE. if layout.align() > bindings::PAGE_SIZE { pr_warn!("KVmalloc does not support alignments larger than PAGE_SIZE yet.\n"); -- cgit From 22ab0641b939967f630d108e33a3582841ad6846 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 31 Jul 2025 17:48:07 +0200 Subject: rust: drm: ensure kmalloc() compatible Layout drm::Device is allocated through __drm_dev_alloc() (which uses kmalloc()) and the driver private data, ::Data, is initialized in-place. Due to the order of fields in drm::Device pub struct Device { dev: Opaque, data: T::Data, } even with an arbitrary large alignment requirement of T::Data it can't happen that the size of Device is smaller than its alignment requirement. However, let's not rely on this subtle circumstance and create a proper kmalloc() compatible Layout. Fixes: 1e4b8896c0f3 ("rust: drm: add device abstraction") Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250731154919.4132-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 3bb7c83966cf..d19410deaf6c 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -5,6 +5,7 @@ //! C header: [`include/linux/drm/drm_device.h`](srctree/include/linux/drm/drm_device.h) use crate::{ + alloc::allocator::Kmalloc, bindings, device, drm, drm::driver::AllocImpl, error::from_err_ptr, @@ -12,7 +13,7 @@ use crate::{ prelude::*, types::{ARef, AlwaysRefCounted, Opaque}, }; -use core::{mem, ops::Deref, ptr, ptr::NonNull}; +use core::{alloc::Layout, mem, ops::Deref, ptr, ptr::NonNull}; #[cfg(CONFIG_DRM_LEGACY)] macro_rules! drm_legacy_fields { @@ -96,6 +97,10 @@ impl Device { /// Create a new `drm::Device` for a `drm::Driver`. pub fn new(dev: &device::Device, data: impl PinInit) -> Result> { + // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()` + // compatible `Layout`. + let layout = Kmalloc::aligned_layout(Layout::new::()); + // SAFETY: // - `VTABLE`, as a `const` is pinned to the read-only section of the compilation, // - `dev` is valid by its type invarants, @@ -103,7 +108,7 @@ impl Device { bindings::__drm_dev_alloc( dev.as_raw(), &Self::VTABLE, - mem::size_of::(), + layout.size(), mem::offset_of!(Self, dev), ) } -- cgit From 0c04a81c1d0214d5b2025f805ccec1ac37c96b08 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 31 Jul 2025 17:48:08 +0200 Subject: rust: drm: remove pin annotations from drm::Device The #[pin_data] and #[pin] annotations are not necessary for drm::Device, since we don't use any pin-init macros, but only __pinned_init() on the impl PinInit argument of drm::Device::new(). Fixes: 1e4b8896c0f3 ("rust: drm: add device abstraction") Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250731154919.4132-4-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 2 -- 1 file changed, 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index d19410deaf6c..d0a9528121f1 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -54,10 +54,8 @@ macro_rules! drm_legacy_fields { /// /// `self.dev` is a valid instance of a `struct device`. #[repr(C)] -#[pin_data] pub struct Device { dev: Opaque, - #[pin] data: T::Data, } -- cgit From 360077278ba62e81310080f075a1a3028e778ef9 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 31 Jul 2025 17:48:09 +0200 Subject: rust: drm: don't pass the address of drm::Device to drm_dev_put() In drm_dev_put() call in AlwaysRefCounted::dec_ref() we rely on struct drm_device to be the first field in drm::Device, whereas everywhere else we correctly obtain the address of the actual struct drm_device. Analogous to the from_drm_device() helper, provide the into_drm_device() helper in order to address this. Fixes: 1e4b8896c0f3 ("rust: drm: add device abstraction") Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250731154919.4132-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index d0a9528121f1..d29c477e89a8 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -120,9 +120,13 @@ impl Device { // - `raw_data` is a valid pointer to uninitialized memory. // - `raw_data` will not move until it is dropped. unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| { - // SAFETY: `__drm_dev_alloc()` was successful, hence `raw_drm` must be valid and the + // SAFETY: `raw_drm` is a valid pointer to `Self`, given that `__drm_dev_alloc` was + // successful. + let drm_dev = unsafe { Self::into_drm_device(raw_drm) }; + + // SAFETY: `__drm_dev_alloc()` was successful, hence `drm_dev` must be valid and the // refcount must be non-zero. - unsafe { bindings::drm_dev_put(ptr::addr_of_mut!((*raw_drm.as_ptr()).dev).cast()) }; + unsafe { bindings::drm_dev_put(drm_dev) }; })?; // SAFETY: The reference count is one, and now we take ownership of that reference as a @@ -143,6 +147,14 @@ impl Device { unsafe { crate::container_of!(Opaque::cast_from(ptr), Self, dev) }.cast_mut() } + /// # Safety + /// + /// `ptr` must be a valid pointer to `Self`. + unsafe fn into_drm_device(ptr: NonNull) -> *mut bindings::drm_device { + // SAFETY: By the safety requirements of this function, `ptr` is a valid pointer to `Self`. + unsafe { &raw mut (*ptr.as_ptr()).dev }.cast() + } + /// Not intended to be called externally, except via declare_drm_ioctls!() /// /// # Safety @@ -192,8 +204,11 @@ unsafe impl AlwaysRefCounted for Device { } unsafe fn dec_ref(obj: NonNull) { + // SAFETY: `obj` is a valid pointer to `Self`. + let drm_dev = unsafe { Self::into_drm_device(obj) }; + // SAFETY: The safety requirements guarantee that the refcount is non-zero. - unsafe { bindings::drm_dev_put(obj.cast().as_ptr()) }; + unsafe { bindings::drm_dev_put(drm_dev) }; } } -- cgit From 0f580d5d3d9d9cd0953695cd32e43aac3a946338 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sat, 16 Aug 2025 22:42:15 +0200 Subject: rust: alloc: fix `rusttest` by providing `Cmalloc::aligned_layout` too Commit fde578c86281 ("rust: alloc: replace aligned_size() with Kmalloc::aligned_layout()") provides a public `aligned_layout` function in `Kamlloc`, but not in `Cmalloc`, and thus uses of it will trigger an error in `rusttest`. Such a user appeared in the following commit 22ab0641b939 ("rust: drm: ensure kmalloc() compatible Layout"): error[E0599]: no function or associated item named `aligned_layout` found for struct `alloc::allocator_test::Cmalloc` in the current scope --> rust/kernel/drm/device.rs:100:31 | 100 | let layout = Kmalloc::aligned_layout(Layout::new::()); | ^^^^^^^^^^^^^^ function or associated item not found in `Cmalloc` | ::: rust/kernel/alloc/allocator_test.rs:19:1 | 19 | pub struct Cmalloc; | ------------------ function or associated item `aligned_layout` not found for this struct Thus add an equivalent one for `Cmalloc`. Fixes: fde578c86281 ("rust: alloc: replace aligned_size() with Kmalloc::aligned_layout()") Signed-off-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250816204215.2719559-1-ojeda@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/allocator_test.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/alloc/allocator_test.rs b/rust/kernel/alloc/allocator_test.rs index a3074480bd8d..90dd987d40e4 100644 --- a/rust/kernel/alloc/allocator_test.rs +++ b/rust/kernel/alloc/allocator_test.rs @@ -22,6 +22,17 @@ pub type Kmalloc = Cmalloc; pub type Vmalloc = Kmalloc; pub type KVmalloc = Kmalloc; +impl Cmalloc { + /// Returns a [`Layout`] that makes [`Kmalloc`] fulfill the requested size and alignment of + /// `layout`. + pub fn aligned_layout(layout: Layout) -> Layout { + // Note that `layout.size()` (after padding) is guaranteed to be a multiple of + // `layout.align()` which together with the slab guarantees means that `Kmalloc` will return + // a properly aligned object (see comments in `kmalloc()` for more information). + layout.pad_to_align() + } +} + extern "C" { #[link_name = "aligned_alloc"] fn libc_aligned_alloc(align: usize, size: usize) -> *mut crate::ffi::c_void; -- cgit