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

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@xxxxxxxxxx> writes:

> "Younes Akhouayri via B4 Relay" <devnull+git.younes.io@xxxxxxxxxx>
> writes:
>
>> 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>
>
>
> Reviewed-by: Andreas Hindborg <a.hindborg@xxxxxxxxxx>
>
> 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