[PATCH v3] rust: configfs: fix object initialization cleanup

From: Younes Akhouayri <[email protected]>

Subsystem::new() calls configfs_register_subsystem() at the end of its
pin initializer. If registration returns an error, release the config
item's initial reference and destroy the initialized mutex before
returning. Otherwise, long subsystem names allocated by
config_item_set_name() leak.

The initial reference also remains after a successful subsystem is
unregistered. Release it from PinnedDrop before the Rust container is
destroyed.

Initialize driver data before the C configfs object in Subsystem::new()
and Group::new(). Then failure while initializing driver data cannot
leave an initialized config group, and its allocated name, behind.

Keeping registration inside try_pin_init! also means PinnedDrop is
installed only after registration succeeds. A duplicate name therefore
returns -EEXIST without attempting to unregister a subsystem whose
ci_dentry was never set.

Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
Signed-off-by: Younes Akhouayri <[email protected]>
---
Changes in v3:
- Initialize driver data before configfs groups.
- Release the initial group reference after registration failure and unregister.
- Link to v2: https://patch.msgid.link/20260818-fix-rust-configfs-registration-state-v1-v2-1-9acedd3070f7@younes.io

Changes in v2:
- Register the subsystem at the end of try_pin_init!.
- Destroy su_mutex when registration fails.
- Remove the registered flag.
- Link to v1: https://patch.msgid.link/20260818-fix-rust-configfs-registration-state-v1-v1-1-c929990bc8ef@younes.io

To: Andreas Hindborg <[email protected]>
To: Breno Leitao <[email protected]>
To: Miguel Ojeda <[email protected]>
To: Boqun Feng <[email protected]>
To: Gary Guo <[email protected]>
To: Björn Roy Baron <[email protected]>
To: Benno Lossin <[email protected]>
To: Alice Ryhl <[email protected]>
To: Trevor Gross <[email protected]>
To: Danilo Krummrich <[email protected]>
To: Daniel Almeida <[email protected]>
To: Tamir Duberstein <[email protected]>
To: Alexandre Courbot <[email protected]>
To: Onur Özkan <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
 rust/kernel/configfs.rs | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index cd082b83e9e7..d28ac9a648f1 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -150,6 +150,7 @@ pub fn new(
         data: impl PinInit<Data, Error>,
     ) -> impl PinInit<Self, Error> {
         try_pin_init!(Self {
+            data <- data,
             subsystem <- pin_init::init_zeroed().chain(
                 |place: &mut Opaque<bindings::configfs_subsystem>| {
                     // SAFETY: We initialized the required fields of `place.group` above.
@@ -172,13 +173,23 @@ pub fn new(
                     Ok(())
                 }
             ),
-            data <- data,
-        })
-        .pin_chain(|this| {
-            crate::error::to_result(
-                // SAFETY: We initialized `this.subsystem` according to C API contract above.
-                unsafe { bindings::configfs_register_subsystem(this.subsystem.get()) },
-            )
+            _: {
+                let result = crate::error::to_result(
+                    // SAFETY: We initialized `subsystem` according to the C API contract above.
+                    unsafe { bindings::configfs_register_subsystem(subsystem.get()) },
+                );
+                if result.is_err() {
+                    // SAFETY: The group and mutex were initialized above, and registration
+                    // failed, so configfs does not hold references to the group.
+                    unsafe {
+                        bindings::config_item_put(
+                            &raw mut (*subsystem.get()).su_group.cg_item,
+                        );
+                        bindings::mutex_destroy(&raw mut (*subsystem.get()).su_mutex);
+                    }
+                }
+                result?
+            }
         })
     }
 }
@@ -188,8 +199,12 @@ impl<Data> PinnedDrop for Subsystem<Data> {
     fn drop(self: Pin<&mut Self>) {
         // SAFETY: We registered `self.subsystem` in the initializer returned by `Self::new`.
         unsafe { bindings::configfs_unregister_subsystem(self.subsystem.get()) };
-        // SAFETY: We initialized the mutex in `Subsystem::new`.
-        unsafe { bindings::mutex_destroy(&raw mut (*self.subsystem.get()).su_mutex) };
+        // SAFETY: Unregistering drops configfs's references to the group, so it is safe to drop
+        // the initial group reference and destroy the initialized mutex.
+        unsafe {
+            bindings::config_item_put(&raw mut (*self.subsystem.get()).su_group.cg_item);
+            bindings::mutex_destroy(&raw mut (*self.subsystem.get()).su_mutex);
+        }
     }
 }
 
@@ -260,6 +275,7 @@ pub fn new(
         data: impl PinInit<Data, Error>,
     ) -> impl PinInit<Self, Error> {
         try_pin_init!(Self {
+            data <- data,
             group <- pin_init::init_zeroed().chain(|v: &mut Opaque<bindings::config_group>| {
                 let place = v.get();
                 let name = name.to_bytes_with_nul().as_ptr();
@@ -269,7 +285,6 @@ pub fn new(
                 };
                 Ok(())
             }),
-            data <- data,
         })
     }
 }

---
base-commit: 47f27155f17498fccb1f222f79089642337498a9
change-id: 20260817-fix-rust-configfs-registration-state-v1-fa33fcc69673

Best regards,
--  
Younes Akhouayri <[email protected]>