mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] rust: configfs: fix data offset calculation for subsystem callbacks
@ 2026-06-08 12:32 Andreas Hindborg
  2026-06-09 14:26 ` Miguel Ojeda
  0 siblings, 1 reply; 2+ messages in thread
From: Andreas Hindborg @ 2026-06-08 12:32 UTC (permalink / raw)
  To: Breno Leitao, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: rust-for-linux, linux-kernel, Andreas Hindborg

`get_group_data()` chooses between `Group::<Parent>::container_of()` and
`Subsystem::<Parent>::container_of()` based on whether `this` represents
the root group of a configfs subsystem. It detects this by checking
`(*this).cg_subsys.is_null()`, but `link_group()` in `fs/configfs/dir.c`
unconditionally sets `cg_subsys` for every `config_group` attached anywhere
in a registered subsystem, including the subsystem's own `su_group`. The
only `config_group` with a NULL `cg_subsys` is the configfs root, on which
userspace cannot trigger callbacks. The check is therefore always false at
runtime, and the `Subsystem` branch is dead. Subsystem-level callbacks
reach the `Group` branch and may read `data` at the wrong offset.

Thus change the `is_root` check to correctly identify whether a group is a
root group by comparing `this` to `subsys.su_group`.

Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/configfs.rs | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 2339c6467325..eb1316154890 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -297,26 +297,36 @@ unsafe fn container_of(group: *const bindings::config_group) -> *const Self {
 ///
 /// `this` must be a valid pointer.
 ///
-/// If `this` does not represent the root group of a configfs subsystem,
-/// `this` must be a pointer to a `bindings::config_group` embedded in a
-/// `Group<Parent>`.
+/// If `this` is the `su_group` field of a `bindings::configfs_subsystem`, that
+/// `configfs_subsystem` must be embedded in a `Subsystem<Parent>`.
 ///
-/// Otherwise, `this` must be a pointer to a `bindings::config_group` that
-/// is embedded in a `bindings::configfs_subsystem` that is embedded in a
-/// `Subsystem<Parent>`.
+/// Otherwise, `this` must be a pointer to a `bindings::config_group` embedded
+/// in a `Group<Parent>`.
 unsafe fn get_group_data<'a, Parent>(this: *mut bindings::config_group) -> &'a Parent {
     // SAFETY: `this` is a valid pointer.
-    let is_root = unsafe { (*this).cg_subsys.is_null() };
-
-    if !is_root {
-        // SAFETY: By C API contact,`this` was returned from a call to
-        // `make_group`. The pointer is known to be embedded within a
-        // `Group<Parent>`.
-        unsafe { &(*Group::<Parent>::container_of(this)).data }
-    } else {
-        // SAFETY: By C API contract, `this` is a pointer to the
-        // `bindings::config_group` field within a `Subsystem<Parent>`.
+    let subsys = unsafe { (*this).cg_subsys };
+    // `link_group()` in `fs/configfs/dir.c` assigns `cg_subsys` for every
+    // `config_group` attached anywhere in a registered subsystem, including
+    // the subsystem's own `su_group` (which gets a pointer to itself). The
+    // only `config_group` with a NULL `cg_subsys` is the configfs root, and
+    // userspace cannot trigger callbacks on it. The group is therefore the
+    // subsystem's `su_group` iff it equals `&cg_subsys->su_group`.
+    //
+    // SAFETY: For every `config_group` the configfs core dispatches a
+    // callback on, `cg_subsys` was set by `link_group()` at registration time
+    // and points to a valid `configfs_subsystem` that outlives the callback.
+    let is_root = !subsys.is_null()
+        && core::ptr::eq(this.cast_const(), unsafe { &raw const (*subsys).su_group });
+
+    if is_root {
+        // SAFETY: By the above, `this` is the `su_group` field of a
+        // `configfs_subsystem` that, by function safety requirements, is
+        // embedded in a `Subsystem<Parent>`.
         unsafe { &(*Subsystem::container_of(this)).data }
+    } else {
+        // SAFETY: By function safety requirements, `this` is a
+        // `config_group` embedded in a `Group<Parent>`.
+        unsafe { &(*Group::<Parent>::container_of(this)).data }
     }
 }
 

---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260608-configfs-fix-offset-6b3117158901

Best regards,
--  
Andreas Hindborg <a.hindborg@kernel.org>



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

* Re: [PATCH] rust: configfs: fix data offset calculation for subsystem callbacks
  2026-06-08 12:32 [PATCH] rust: configfs: fix data offset calculation for subsystem callbacks Andreas Hindborg
@ 2026-06-09 14:26 ` Miguel Ojeda
  0 siblings, 0 replies; 2+ messages in thread
From: Miguel Ojeda @ 2026-06-09 14:26 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Breno Leitao, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, rust-for-linux, linux-kernel

On Mon, Jun 8, 2026 at 2:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")

Same here: should this have Cc: stable given the hash? i.e. 6.18.y+

Cheers,
Miguel

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

end of thread, other threads:[~2026-06-09 14:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-08 12:32 [PATCH] rust: configfs: fix data offset calculation for subsystem callbacks Andreas Hindborg
2026-06-09 14:26 ` Miguel Ojeda

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®