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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87il1zs7vl.fsf@nvidia.com>
Date: Wed, 06 Mar 2024 07:18:35 -0800
From: Rahul Rameshbabu <rrameshbabu@...dia.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: netdev@...r.kernel.org, arkadiusz.kubalewski@...el.com,
 vadim.fedorenko@...ux.dev, milena.olech@...el.com
Subject: Re: [patch net] dpll: fix dpll_xa_ref_*_del() for multiple
 registrations

On Wed, 06 Mar, 2024 16:12:40 +0100 Jiri Pirko <jiri@...nulli.us> wrote:
> From: Jiri Pirko <jiri@...dia.com>
>
> 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 <jiri@...dia.com>
> ---
>  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);