netdev - Re: [net-next,v2,08/12] dpll: Enhance and consolidate reference counting logic

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: <20260121001650.1904392-2-kuba@kernel.org>
Date: Tue, 20 Jan 2026 16:16:51 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: ivecera@...hat.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	conor+dt@...nel.org,
	poros@...hat.com,
	anthony.l.nguyen@...el.com,
	linux-rdma@...r.kernel.org,
	tariqt@...dia.com,
	robh@...nel.org,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	aleksander.lobakin@...el.com,
	mbloch@...dia.com,
	jiri@...nulli.us,
	Prathosh.Satish@...rochip.com,
	krzk+dt@...nel.org,
	saeedm@...dia.com,
	devicetree@...r.kernel.org,
	davem@...emloft.net,
	pabeni@...hat.com,
	przemyslaw.kitszel@...el.com,
	arkadiusz.kubalewski@...el.com,
	jonathan.lemon@...il.com,
	saravanak@...nel.org,
	aleksandr.loktionov@...el.com,
	mschmidt@...hat.com,
	edumazet@...gle.com,
	leon@...nel.org,
	vadim.fedorenko@...ux.dev,
	grzegorz.nitka@...el.com,
	intel-wired-lan@...ts.osuosl.org,
	richardcochran@...il.com,
	andrew+netdev@...n.ch
Subject: Re: [net-next,v2,08/12] dpll: Enhance and consolidate reference counting logic

This is an AI-generated review of your patch.

Dunno if there's a reason for having this fixed by a later patch,
if not let's fix. I'm sending the review mostly because of the
comments on patch 12.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
dpll: Enhance and consolidate reference counting logic

This commit refactors DPLL reference counting by introducing centralized
__dpll_{device,pin}_{hold,put}() helpers and updating dpll_xa_ref_*
functions to automatically manage references when adding/removing entries.

> diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
> index 156f95de8e8e..f2a77eb1b991 100644

[ ... ]

> @@ -155,6 +194,7 @@ dpll_xa_ref_pin_add(struct xarray *xa_pins, struct dpll_pin *pin,
>  	reg->ops = ops;
>  	reg->priv = priv;
>  	reg->cookie = cookie;
> +	__dpll_pin_hold(pin);
>  	if (ref_exists)
>  		refcount_inc(&ref->refcount);
>  	list_add_tail(&reg->list, &ref->registration_list);

The new __dpll_pin_hold(pin) call is added here for the success path, but
looking at the context above this hunk, there is an early return path when
a duplicate registration is found:

    reg = dpll_pin_registration_find(ref, ops, priv, cookie);
    if (reg) {
        refcount_inc(&ref->refcount);
        return 0;
    }

This early return path increments ref->refcount and returns success, but
does not call __dpll_pin_hold(pin). Meanwhile, dpll_xa_ref_pin_del() always
calls __dpll_pin_put(pin). Could this cause a refcount underflow if a caller
attempts duplicate registration and then later unregistration is attempted?

Note: This appears to be addressed by 934c22045c2a ("dpll: Prevent duplicate
registrations") later in the series, which changes this early return to
return -EEXIST instead of success.

[ ... ]

> @@ -236,6 +277,7 @@ dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct dpll_device *dpll,
>  	reg->ops = ops;
>  	reg->priv = priv;
>  	reg->cookie = cookie;
> +	__dpll_device_hold(dpll);
>  	if (ref_exists)
>  		refcount_inc(&ref->refcount);
>  	list_add_tail(&reg->list, &ref->registration_list);

The same pattern exists in dpll_xa_ref_dpll_add(): the early return path for
duplicate registrations increments ref->refcount without calling
__dpll_device_hold(dpll), while dpll_xa_ref_dpll_del() always calls
__dpll_device_put(dpll). The same fix in 934c22045c2a addresses this.
-- 
pw-bot: cr