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

"Gary Guo" <[email protected]> writes:

> On Tue Aug 18, 2026 at 11:53 AM BST, Andreas Hindborg wrote:
>> "Gary Guo" <[email protected]> writes:
>>
>>> On Tue Aug 18, 2026 at 9:11 AM BST, Younes Akhouayri via B4 Relay wrote:
>>>> From: Younes Akhouayri <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> ---
>>>>  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