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@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.

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@xxxxxxxxx>