Re: [PATCH] rust: configfs: skip unregister after failed registration
From: Gary Guo
Date: Tue Aug 18 2026 - 07:01:11 EST
On Tue Aug 18, 2026 at 11:53 AM BST, Andreas Hindborg wrote:
> "Gary Guo" <gary@xxxxxxxxxxx> writes:
>
>> On Tue Aug 18, 2026 at 9:11 AM BST, Younes Akhouayri via B4 Relay wrote:
>>> From: Younes Akhouayri <git@xxxxxxxxx>
>>>
>>> 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@xxxxxxxxx>
>>> ---
>>> 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