Re: [PATCH 3/4] rust: configfs: introduce rust support for configfs

Andreas Hindborg
2025-02-06T12:34:09Z

"Andreas Hindborg" <a.hindborg@kernel.org> writes:

> This patch adds a rust API for configfs, thus allowing rust modules to use
> configfs for configuration. The implementation is a shim on top of the C
> configfs implementation allowing safe use of the C infrastructure from
> rust.
>
> The patch enables the `const_mut_refs` feature on compilers before rustc
> 1.83. The feature was stabilized in rustc 1.83 and is not required to be
> explicitly enabled on later versions.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> ---

[...]

> + /// # Safety
> + ///
> + /// If `this` does not represent the root group of a `configfs` subsystem,
> + /// `this` must be a pointer to a `bindings::config_group` embedded in a
> + /// `Group<PAR>`.
> + ///
> + /// Otherwise, `this` must be a pointer to a `bindings::config_group` that
> + /// is embedded in a `bindings::configfs_subsystem` that is embedded in a
> + /// `Subsystem<PAR>`.
> + ///
> + /// `item` must point to a `bindings::config_item` within a
> + /// `bindings::config_group` within a `Group<CHLD>`.
> + unsafe extern "C" fn drop_item(
> + this: *mut bindings::config_group,
> + item: *mut bindings::config_item,
> + ) {
> + // SAFETY: By function safety requirements of this function, this call
> + // is safe.
> + let parent_data = unsafe { get_group_data(this) };
> +
> + // SAFETY: By function safety requirements, `item` is embedded in a
> + // `config_group`.
> + let c_child_group_ptr =
> + unsafe { kernel::container_of!(item, bindings::config_group, cg_item) };
> + // SAFETY: By function safety requirements, `c_child_group_ptr` is
> + // embedded within a `Group<CHLD>`.
> + let r_child_group_ptr = unsafe { Group::<CHLD>::container_of(c_child_group_ptr) };
> +
> + if PAR::HAS_DROP_ITEM {
> + PAR::drop_item(
> + parent_data,
> + // SAFETY: We called `into_foreign` to produce `r_child_group_ptr` in
> + // `make_group`. There are not other borrows of this pointer in existence.
> + unsafe { PCPTR::borrow(r_child_group_ptr.cast_mut()) },
> + );
> + }
> +
> + // SAFETY: By C API contract, `configfs` is not going to touch `item`
> + // again.
> + unsafe { bindings::config_item_put(item) };

This turned out to be wrong. We _do_ have to let go of a refcount here,
but we are not allowed to free the item.

> +
> + // SAFETY: We called `into_foreign` on `r_chilc_group_ptr` in
> + // `make_group`.
> + let pin_child: PCPTR = unsafe { PCPTR::from_foreign(r_child_group_ptr.cast_mut()) };
> + drop(pin_child);

So this is wrong and will cause UAF. We have to wait for a call to
ct_item_ops.release and do the cleanup there. I will address this in the
next version. Removing directories is likely to cause trouble with this
patch.

Best regards,
Andreas Hindborg