[PATCH] rust: configfs: skip unregister after failed registration

[PATCH] rust: configfs: skip unregister after failed registration

[Younes Akhouayri via B4 Relay]

From: Younes Akhouayri <git@younes.io>

Subsystem::new() calls configfs_register_subsystem() from a fallible
pin_chain callback. If registration fails, ChainPinInit drops the
already initialized Subsystem. Its PinnedDrop currently calls
configfs_unregister_subsystem() unconditionally.

configfs_unregister_subsystem() requires registration to have completed
and immediately dereferences the subsystem dentry. Registering a
duplicate subsystem name returns -EEXIST before installing that dentry,
so the cleanup path dereferences NULL and panics the kernel.

Track successful registration explicitly and only unregister in that
state. Keep mutex destruction unconditional because it is initialized
before registration.

Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
Signed-off-by: Younes Akhouayri <git@younes.io>
---
 rust/kernel/configfs.rs | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index cd082b83e9e7..f358e227ce09 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -130,6 +130,7 @@ pub struct Subsystem<Data> {
     subsystem: Opaque<bindings::configfs_subsystem>,
     #[pin]
     data: Data,
+    registered: bool,
 }
 
 // SAFETY: We do not provide any operations on `Subsystem`.
@@ -173,12 +174,15 @@ pub fn new(
                 }
             ),
             data <- data,
+            registered: false,
         })
-        .pin_chain(|this| {
+        .pin_chain(|mut this| {
             crate::error::to_result(
                 // SAFETY: We initialized `this.subsystem` according to C API contract above.
                 unsafe { bindings::configfs_register_subsystem(this.subsystem.get()) },
-            )
+            )?;
+            *this.as_mut().project().registered = true;
+            Ok(())
         })
     }
 }
@@ -186,8 +190,10 @@ pub fn new(
 #[pinned_drop]
 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()) };
+        if self.registered {
+            // SAFETY: `registered` is only set after `self.subsystem` was registered.
+            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) };
     }

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

Best regards,
--  
Younes Akhouayri <git@younes.io>

Re: [PATCH] rust: configfs: skip unregister after failed registration

[Andreas Hindborg]

"Younes Akhouayri via B4 Relay" <devnull+git.younes.io@kernel.org>
writes:

> From: Younes Akhouayri <git@younes.io>
>
> Subsystem::new() calls configfs_register_subsystem() from a fallible
> pin_chain callback. If registration fails, ChainPinInit drops the
> already initialized Subsystem. Its PinnedDrop currently calls
> configfs_unregister_subsystem() unconditionally.
>
> configfs_unregister_subsystem() requires registration to have completed
> and immediately dereferences the subsystem dentry. Registering a
> duplicate subsystem name returns -EEXIST before installing that dentry,
> so the cleanup path dereferences NULL and panics the kernel.
>
> Track successful registration explicitly and only unregister in that
> state. Keep mutex destruction unconditional because it is initialized
> before registration.
>
> Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
> Signed-off-by: Younes Akhouayri <git@younes.io>


Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>

Looks good to me, will pick it in a few weeks.

Best regards,
Andreas Hindborg

Re: [PATCH] rust: configfs: skip unregister after failed registration

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> "Younes Akhouayri via B4 Relay" <devnull+git.younes.io@kernel.org>
> writes:
>
>> From: Younes Akhouayri <git@younes.io>
>>
>> Subsystem::new() calls configfs_register_subsystem() from a fallible
>> pin_chain callback. If registration fails, ChainPinInit drops the
>> already initialized Subsystem. Its PinnedDrop currently calls
>> configfs_unregister_subsystem() unconditionally.
>>
>> configfs_unregister_subsystem() requires registration to have completed
>> and immediately dereferences the subsystem dentry. Registering a
>> duplicate subsystem name returns -EEXIST before installing that dentry,
>> so the cleanup path dereferences NULL and panics the kernel.
>>
>> Track successful registration explicitly and only unregister in that
>> state. Keep mutex destruction unconditional because it is initialized
>> before registration.
>>
>> Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
>> Signed-off-by: Younes Akhouayri <git@younes.io>
>
>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Looks good to me, will pick it in a few weeks.

Actually, we do not need the `as_mut` call:

diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index fce994e7f8b8..62fd2d458a1c 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -176,12 +176,12 @@ pub fn new(
             data <- data,
             registered: false,
         })
-        .pin_chain(|mut this| {
+        .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()) },
             )?;
-            *this.as_mut().project().registered = true;
+            *this.project().registered = true;
             Ok(())
         })
     }


Best regards,
Andreas Hindborg

Re: [PATCH] rust: configfs: skip unregister after failed registration

[Gary Guo]

On Tue Aug 18, 2026 at 9:11 AM BST, Younes Akhouayri via B4 Relay wrote:
> From: Younes Akhouayri <git@younes.io>
>
> Subsystem::new() calls configfs_register_subsystem() from a fallible
> pin_chain callback. If registration fails, ChainPinInit drops the
> already initialized Subsystem. Its PinnedDrop currently calls
> configfs_unregister_subsystem() unconditionally.
>
> configfs_unregister_subsystem() requires registration to have completed
> and immediately dereferences the subsystem dentry. Registering a
> duplicate subsystem name returns -EEXIST before installing that dentry,
> so the cleanup path dereferences NULL and panics the kernel.
>
> Track successful registration explicitly and only unregister in that
> state. Keep mutex destruction unconditional because it is initialized
> before registration.
>
> Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
> Signed-off-by: Younes Akhouayri <git@younes.io>
> ---
>  rust/kernel/configfs.rs | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
> index cd082b83e9e7..f358e227ce09 100644
> --- a/rust/kernel/configfs.rs
> +++ b/rust/kernel/configfs.rs
> @@ -130,6 +130,7 @@ pub struct Subsystem<Data> {
>      subsystem: Opaque<bindings::configfs_subsystem>,
>      #[pin]
>      data: Data,
> +    registered: bool,

No flag just for destruction. Please change new logic to avoid needing this.

Best,
Gary

>  }
>  
>  // SAFETY: We do not provide any operations on `Subsystem`.
> @@ -173,12 +174,15 @@ pub fn new(
>                  }
>              ),
>              data <- data,
> +            registered: false,
>          })
> -        .pin_chain(|this| {
> +        .pin_chain(|mut this| {
>              crate::error::to_result(
>                  // SAFETY: We initialized `this.subsystem` according to C API contract above.
>                  unsafe { bindings::configfs_register_subsystem(this.subsystem.get()) },
> -            )
> +            )?;
> +            *this.as_mut().project().registered = true;
> +            Ok(())
>          })
>      }
>  }
> @@ -186,8 +190,10 @@ pub fn new(
>  #[pinned_drop]
>  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()) };
> +        if self.registered {
> +            // SAFETY: `registered` is only set after `self.subsystem` was registered.
> +            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) };
>      }
>
> ---
> base-commit: 47f27155f17498fccb1f222f79089642337498a9
> change-id: 20260817-fix-rust-configfs-registration-state-v1-fa33fcc69673
>
> Best regards,
> --  
> Younes Akhouayri <git@younes.io>

Re: [PATCH] rust: configfs: skip unregister after failed registration

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> On Tue Aug 18, 2026 at 9:11 AM BST, Younes Akhouayri via B4 Relay wrote:
>> From: Younes Akhouayri <git@younes.io>
>>
>> Subsystem::new() calls configfs_register_subsystem() from a fallible
>> pin_chain callback. If registration fails, ChainPinInit drops the
>> already initialized Subsystem. Its PinnedDrop currently calls
>> configfs_unregister_subsystem() unconditionally.
>>
>> configfs_unregister_subsystem() requires registration to have completed
>> and immediately dereferences the subsystem dentry. Registering a
>> duplicate subsystem name returns -EEXIST before installing that dentry,
>> so the cleanup path dereferences NULL and panics the kernel.
>>
>> Track successful registration explicitly and only unregister in that
>> state. Keep mutex destruction unconditional because it is initialized
>> before registration.
>>
>> Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
>> Signed-off-by: Younes Akhouayri <git@younes.io>
>> ---
>>  rust/kernel/configfs.rs | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
>> index cd082b83e9e7..f358e227ce09 100644
>> --- a/rust/kernel/configfs.rs
>> +++ b/rust/kernel/configfs.rs
>> @@ -130,6 +130,7 @@ pub struct Subsystem<Data> {
>>      subsystem: Opaque<bindings::configfs_subsystem>,
>>      #[pin]
>>      data: Data,
>> +    registered: bool,
>
> No flag just for destruction. Please change new logic to avoid needing this.

I guess we can have a local `UnregisteredSubsystem` that we can
cast to a `Subsystem` once registration succeeds. Is that what you have
in mind?


Best regards,
Andreas Hindborg

Re: [PATCH] rust: configfs: skip unregister after failed registration

[Gary Guo]

On Tue Aug 18, 2026 at 11:53 AM BST, Andreas Hindborg wrote:
> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Tue Aug 18, 2026 at 9:11 AM BST, Younes Akhouayri via B4 Relay wrote:
>>> From: Younes Akhouayri <git@younes.io>
>>>
>>> Subsystem::new() calls configfs_register_subsystem() from a fallible
>>> pin_chain callback. If registration fails, ChainPinInit drops the
>>> already initialized Subsystem. Its PinnedDrop currently calls
>>> configfs_unregister_subsystem() unconditionally.
>>>
>>> configfs_unregister_subsystem() requires registration to have completed
>>> and immediately dereferences the subsystem dentry. Registering a
>>> duplicate subsystem name returns -EEXIST before installing that dentry,
>>> so the cleanup path dereferences NULL and panics the kernel.
>>>
>>> Track successful registration explicitly and only unregister in that
>>> state. Keep mutex destruction unconditional because it is initialized
>>> before registration.
>>>
>>> Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
>>> Signed-off-by: Younes Akhouayri <git@younes.io>
>>> ---
>>>  rust/kernel/configfs.rs | 14 ++++++++++----
>>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
>>> index cd082b83e9e7..f358e227ce09 100644
>>> --- a/rust/kernel/configfs.rs
>>> +++ b/rust/kernel/configfs.rs
>>> @@ -130,6 +130,7 @@ pub struct Subsystem<Data> {
>>>      subsystem: Opaque<bindings::configfs_subsystem>,
>>>      #[pin]
>>>      data: Data,
>>> +    registered: bool,
>>
>> No flag just for destruction. Please change new logic to avoid needing this.
>
> I guess we can have a local `UnregisteredSubsystem` that we can
> cast to a `Subsystem` once registration succeeds. Is that what you have
> in mind?

You can use the arbitrary code block feature of pin-init to run code before
arming the destructor:

    _: {
        let result = crate::error::to_result(
            unsafe { bindings::configfs_register_subsystem(subsystem.get()) }
        );
        if let Err(err) = result {
            unsafe { bindings::mutex_destroy(&raw mut (*subsystem.get()).su_mutex) };
        }
        result?
    }

That said, why is the configfs not initializer the mutex, but rather users do?

Best,
Gary

Re: [PATCH] rust: configfs: skip unregister after failed registration

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> On Tue Aug 18, 2026 at 11:53 AM BST, Andreas Hindborg wrote:
>> "Gary Guo" <gary@garyguo.net> writes:
>>
>>> On Tue Aug 18, 2026 at 9:11 AM BST, Younes Akhouayri via B4 Relay wrote:
>>>> From: Younes Akhouayri <git@younes.io>
>>>>
>>>> Subsystem::new() calls configfs_register_subsystem() from a fallible
>>>> pin_chain callback. If registration fails, ChainPinInit drops the
>>>> already initialized Subsystem. Its PinnedDrop currently calls
>>>> configfs_unregister_subsystem() unconditionally.
>>>>
>>>> configfs_unregister_subsystem() requires registration to have completed
>>>> and immediately dereferences the subsystem dentry. Registering a
>>>> duplicate subsystem name returns -EEXIST before installing that dentry,
>>>> so the cleanup path dereferences NULL and panics the kernel.
>>>>
>>>> Track successful registration explicitly and only unregister in that
>>>> state. Keep mutex destruction unconditional because it is initialized
>>>> before registration.
>>>>
>>>> Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
>>>> Signed-off-by: Younes Akhouayri <git@younes.io>
>>>> ---
>>>>  rust/kernel/configfs.rs | 14 ++++++++++----
>>>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
>>>> index cd082b83e9e7..f358e227ce09 100644
>>>> --- a/rust/kernel/configfs.rs
>>>> +++ b/rust/kernel/configfs.rs
>>>> @@ -130,6 +130,7 @@ pub struct Subsystem<Data> {
>>>>      subsystem: Opaque<bindings::configfs_subsystem>,
>>>>      #[pin]
>>>>      data: Data,
>>>> +    registered: bool,
>>>
>>> No flag just for destruction. Please change new logic to avoid needing this.
>>
>> I guess we can have a local `UnregisteredSubsystem` that we can
>> cast to a `Subsystem` once registration succeeds. Is that what you have
>> in mind?
>
> You can use the arbitrary code block feature of pin-init to run code before
> arming the destructor:
>
>     _: {
>         let result = crate::error::to_result(
>             unsafe { bindings::configfs_register_subsystem(subsystem.get()) }
>         );
>         if let Err(err) = result {
>             unsafe { bindings::mutex_destroy(&raw mut (*subsystem.get()).su_mutex) };
>         }
>         result?
>     }

Neat, I did not know about that.

>
> That said, why is the configfs not initializer the mutex, but rather users do?

That is just the way configfs expects users to use the API. C users can
initialize statically as well by assigning the mutex at declaration
time. Try to grep for `init.*su_mutex` at kernel root.

Best regards,
Andreas Hindborg