Re: [PATCH net-next 02/14] net: ethtool: make sure __ethtool_get_link_ksettings() is ops-locked

[Jakub Sitnicki <jakub@cloudflare.com>]

On Thu, May 28, 2026 at 04:16 PM -07, Jakub Kicinski wrote:
> __ethtool_get_link_ksettings() is exported and called from sysfs
> and many drivers. Looks like commit 2bcf4772e45a ("net: ethtool:
> try to protect all callback with netdev instance lock")
> missed adding the lock around it. Not treating this as a fix because
> I don't think any driver cares at this point, but if we want to
> remove the rtnl_lock protection this will become critical.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  include/linux/ethtool.h |  2 ++
>  net/ethtool/ioctl.c     | 20 +++++++++++++++++---
>  net/ethtool/linkinfo.c  |  4 ++--
>  net/ethtool/linkmodes.c |  4 ++--
>  4 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 1cb0740ba331..0caaeb91b094 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -325,6 +325,8 @@ struct ethtool_link_ksettings {
>  extern int
>  __ethtool_get_link_ksettings(struct net_device *dev,
>  			     struct ethtool_link_ksettings *link_ksettings);
> +int ethtool_get_link_ksettings_locked(struct net_device *dev,
> +				      struct ethtool_link_ksettings *link_ksettings);
>  
>  struct ethtool_keee {
>  	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index bd97f9b9bf18..1d74ee67e77a 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -436,10 +436,10 @@ struct ethtool_link_usettings {
>  };
>  
>  /* Internal kernel helper to query a device ethtool_link_settings. */
> -int __ethtool_get_link_ksettings(struct net_device *dev,
> -				 struct ethtool_link_ksettings *link_ksettings)
> +int ethtool_get_link_ksettings_locked(struct net_device *dev,
> +				      struct ethtool_link_ksettings *link_ksettings)
>  {
> -	ASSERT_RTNL();
> +	netdev_ops_assert_locked(dev);

Not sure why we're using the "compat" assertion here, which falls back
to check if RTNL is held, instead of the newly introduced
netdev_assert_locked_if_ops().

The contract here is that all callers are expected to hold the
netdev lock (if needed), IIUC.

[...]