Subject: Re: [patch net] dpll: fix dpll_xa_ref_*_del() for multiple registrations

From: Rahul Rameshbabu <[email-masked]>
Date: Wed, 06 Mar 2024 07:18:35 -0800

On Wed, 06 Mar, 2024 16:12:40 +0100 Jiri Pirko <[email-masked]> wrote:
> From: Jiri Pirko <[email-masked]>
>
> Currently, if there are multiple registrations of the same pin on the
> same dpll device, following warnings are observed:
> WARNING: CPU: 5 PID: 2212 at drivers/dpll/dpll_core.c:143 dpll_xa_ref_pin_del.isra.0+0x21e/0x230
> WARNING: CPU: 5 PID: 2212 at drivers/dpll/dpll_core.c:223 __dpll_pin_unregister+0x2b3/0x2c0
>
> The problem is, that in both dpll_xa_ref_dpll_del() and
> dpll_xa_ref_pin_del() registration is only removed from list in case the
> reference count drops to zero. That is wrong, the registration has to
> be removed always.

What about the case where you have two functions/netdevs that refer to
the same DPLL device/pin and you only remove a single function? You have
another function/netdev left that now refers to the unregistered DPLL
device/pin.

>
> To fix this, remove the registration from the list and free
> it unconditionally, instead of doing it only when the ref reference
> counter reaches zero.
>
> Fixes: 9431063ad323 ("dpll: core: Add DPLL framework base functions")
> Signed-off-by: Jiri Pirko <[email-masked]>
> ---
>  drivers/dpll/dpll_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
> index 7f686d179fc9..c751a87c7a8e 100644
> --- a/drivers/dpll/dpll_core.c
> +++ b/drivers/dpll/dpll_core.c
> @@ -129,9 +129,9 @@ static int dpll_xa_ref_pin_del(struct xarray *xa_pins, struct dpll_pin *pin,
>  		reg = dpll_pin_registration_find(ref, ops, priv);
>  		if (WARN_ON(!reg))
>  			return -EINVAL;
> +		list_del(&reg->list);
> +		kfree(reg);
>  		if (refcount_dec_and_test(&ref->refcount)) {
> -			list_del(&reg->list);
> -			kfree(reg);
>  			xa_erase(xa_pins, i);
>  			WARN_ON(!list_empty(&ref->registration_list));
>  			kfree(ref);
> @@ -209,9 +209,9 @@ dpll_xa_ref_dpll_del(struct xarray *xa_dplls, struct dpll_device *dpll,
>  		reg = dpll_pin_registration_find(ref, ops, priv);
>  		if (WARN_ON(!reg))
>  			return;
> +		list_del(&reg->list);
> +		kfree(reg);
>  		if (refcount_dec_and_test(&ref->refcount)) {
> -			list_del(&reg->list);
> -			kfree(reg);
>  			xa_erase(xa_dplls, i);
>  			WARN_ON(!list_empty(&ref->registration_list));
>  			kfree(ref);