linux-kernel - Re: [PATCH net-next v13 02/13] net: pse-pd: Add support for reporting events

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: <20250614121843.427cfc42@kernel.org>
Date: Sat, 14 Jun 2025 12:18:43 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Kory Maincent <kory.maincent@...tlin.com>
Cc: Andrew Lunn <andrew@...n.ch>, Oleksij Rempel <o.rempel@...gutronix.de>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Jonathan Corbet
 <corbet@....net>, Donald Hunter <donald.hunter@...il.com>, Rob Herring
 <robh@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>, Simon Horman
 <horms@...nel.org>, Heiner Kallweit <hkallweit1@...il.com>, Russell King
 <linux@...linux.org.uk>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark
 Brown <broonie@...nel.org>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, netdev@...r.kernel.org,
 linux-doc@...r.kernel.org, Kyle Swenson <kyle.swenson@....tech>, Dent
 Project <dentproject@...uxfoundation.org>, kernel@...gutronix.de, Maxime
 Chevallier <maxime.chevallier@...tlin.com>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v13 02/13] net: pse-pd: Add support for
 reporting events

On Tue, 10 Jun 2025 10:11:36 +0200 Kory Maincent wrote:
> +static struct net_device *
> +pse_control_find_net_by_id(struct pse_controller_dev *pcdev, int id,
> +			   netdevice_tracker *tracker)
> +{
> +	struct pse_control *psec, *next;
> +
> +	mutex_lock(&pse_list_mutex);
> +	list_for_each_entry_safe(psec, next, &pcdev->pse_control_head, list) {

nit: _safe is not necessary here, the body of the if always exits after
dropping the lock

Do you plan to add more callers for this function?
Maybe it's better if it returns the psec pointer with the refcount
elevated. Because it would be pretty neat if we could move the 
ethnl_pse_send_ntf(netdev, notifs, &extack); that  pse_isr() does
right after calling this function under the rtnl_lock.
I don't think calling ethnl_pse_send_ntf() may crash the kernel as is,
but it feels like a little bit of a trap to have ethtool code called
outside of any networking lock.

> +		if (psec->id == id) {
> +			struct net_device *netdev = NULL;
> +			struct phy_device *phydev;
> +
> +			kref_get(&psec->refcnt);
> +			/* Release the mutex before taking the rtnl lock
> +			 * to avoid deadlock in case of a pse_control_put
> +			 * call with the rtnl lock held.
> +			 */
> +			mutex_unlock(&pse_list_mutex);
> +			/* Acquire rtnl to protect the net device
> +			 * reference get.
> +			 */
> +			rtnl_lock();
> +			phydev = psec->attached_phydev;
> +			if (phydev->attached_dev) {
> +				netdev = phydev->attached_dev;
> +				netdev_hold(netdev, tracker, GFP_KERNEL);
> +			}
> +			rtnl_unlock();
> +			pse_control_put(psec);
> +			return netdev;
> +		}
> +	}
> +	mutex_unlock(&pse_list_mutex);
> +	return NULL;
> +}