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

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?
    }

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

Best,
Gary