Commit 2bcf4772 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

net: ethtool: try to protect all callback with netdev instance lock 



Protect all ethtool callbacks and PHY related state with the netdev
instance lock, for drivers which want / need to have their ops
instance-locked. Basically take the lock everywhere we take rtnl_lock.
It was tempting to take the lock in ethnl_ops_begin(), but turns
out we actually nest those calls (when generating notifications).

Tested-by: default avatarMaxime Chevallier <maxime.chevallier@bootlin.com>
Cc: Saeed Mahameed <saeed@kernel.org>
Signed-off-by: default avatarStanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250305163732.2766420-11-sdf@fomichev.me


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 97246d6d

drivers/net/netdevsim/ethtool.c

+0 −2
Original line number Diff line number Diff line
@@ -107,10 +107,8 @@ nsim_set_channels(struct net_device *dev, struct ethtool_channels *ch) 
	struct netdevsim *ns = netdev_priv(dev);

	int err;



	netdev_lock(dev);

	err = netif_set_real_num_queues(dev, ch->combined_count,

					ch->combined_count);

	netdev_unlock(dev);

	if (err)

		return err;

net/dsa/conduit.c

+15 −1
Original line number Diff line number Diff line
@@ -26,7 +26,9 @@ static int dsa_conduit_get_regs_len(struct net_device *dev) 
	int len;



	if (ops->get_regs_len) {

		netdev_lock_ops(dev);

		len = ops->get_regs_len(dev);

		netdev_unlock_ops(dev);

		if (len < 0)

			return len;

		ret += len;
@@ -57,11 +59,15 @@ static void dsa_conduit_get_regs(struct net_device *dev, 
	int len;



	if (ops->get_regs_len && ops->get_regs) {

		netdev_lock_ops(dev);

		len = ops->get_regs_len(dev);

		if (len < 0)

		if (len < 0) {

			netdev_unlock_ops(dev);

			return;

		}

		regs->len = len;

		ops->get_regs(dev, regs, data);

		netdev_unlock_ops(dev);

		data += regs->len;

	}
@@ -91,8 +97,10 @@ static void dsa_conduit_get_ethtool_stats(struct net_device *dev, 
	int count = 0;



	if (ops->get_sset_count && ops->get_ethtool_stats) {

		netdev_lock_ops(dev);

		count = ops->get_sset_count(dev, ETH_SS_STATS);

		ops->get_ethtool_stats(dev, stats, data);

		netdev_unlock_ops(dev);

	}



	if (ds->ops->get_ethtool_stats)
@@ -114,8 +122,10 @@ static void dsa_conduit_get_ethtool_phy_stats(struct net_device *dev, 
		if (count >= 0)

			phy_ethtool_get_stats(dev->phydev, stats, data);

	} else if (ops->get_sset_count && ops->get_ethtool_phy_stats) {

		netdev_lock_ops(dev);

		count = ops->get_sset_count(dev, ETH_SS_PHY_STATS);

		ops->get_ethtool_phy_stats(dev, stats, data);

		netdev_unlock_ops(dev);

	}



	if (count < 0)
@@ -132,11 +142,13 @@ static int dsa_conduit_get_sset_count(struct net_device *dev, int sset) 
	struct dsa_switch *ds = cpu_dp->ds;

	int count = 0;



	netdev_lock_ops(dev);

	if (sset == ETH_SS_PHY_STATS && dev->phydev &&

	    !ops->get_ethtool_phy_stats)

		count = phy_ethtool_get_sset_count(dev->phydev);

	else if (ops->get_sset_count)

		count = ops->get_sset_count(dev, sset);

	netdev_unlock_ops(dev);



	if (count < 0)

		count = 0;
@@ -163,6 +175,7 @@ static void dsa_conduit_get_strings(struct net_device *dev, uint32_t stringset, 
	/* We do not want to be NULL-terminated, since this is a prefix */

	pfx[sizeof(pfx) - 1] = '_';



	netdev_lock_ops(dev);

	if (stringset == ETH_SS_PHY_STATS && dev->phydev &&

	    !ops->get_ethtool_phy_stats) {

		mcount = phy_ethtool_get_sset_count(dev->phydev);
@@ -176,6 +189,7 @@ static void dsa_conduit_get_strings(struct net_device *dev, uint32_t stringset, 
			mcount = 0;

		ops->get_strings(dev, stringset, data);

	}

	netdev_unlock_ops(dev);



	if (ds->ops->get_strings) {

		ndata = data + mcount * len;

net/ethtool/cabletest.c

+12 −8
Original line number Diff line number Diff line
@@ -72,23 +72,24 @@ int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info) 
	dev = req_info.dev;



	rtnl_lock();

	netdev_lock_ops(dev);

	phydev = ethnl_req_get_phydev(&req_info,

				      tb[ETHTOOL_A_CABLE_TEST_HEADER],

				      info->extack);

	if (IS_ERR_OR_NULL(phydev)) {

		ret = -EOPNOTSUPP;

		goto out_rtnl;

		goto out_unlock;

	}



	ops = ethtool_phy_ops;

	if (!ops || !ops->start_cable_test) {

		ret = -EOPNOTSUPP;

		goto out_rtnl;

		goto out_unlock;

	}



	ret = ethnl_ops_begin(dev);

	if (ret < 0)

		goto out_rtnl;

		goto out_unlock;



	ret = ops->start_cable_test(phydev, info->extack);
@@ -97,7 +98,8 @@ int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info) 
	if (!ret)

		ethnl_cable_test_started(phydev, ETHTOOL_MSG_CABLE_TEST_NTF);



out_rtnl:

out_unlock:

	netdev_unlock_ops(dev);

	rtnl_unlock();

	ethnl_parse_header_dev_put(&req_info);

	return ret;
@@ -339,23 +341,24 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info) 
		goto out_dev_put;



	rtnl_lock();

	netdev_lock_ops(dev);

	phydev = ethnl_req_get_phydev(&req_info,

				      tb[ETHTOOL_A_CABLE_TEST_TDR_HEADER],

				      info->extack);

	if (IS_ERR_OR_NULL(phydev)) {

		ret = -EOPNOTSUPP;

		goto out_rtnl;

		goto out_unlock;

	}



	ops = ethtool_phy_ops;

	if (!ops || !ops->start_cable_test_tdr) {

		ret = -EOPNOTSUPP;

		goto out_rtnl;

		goto out_unlock;

	}



	ret = ethnl_ops_begin(dev);

	if (ret < 0)

		goto out_rtnl;

		goto out_unlock;



	ret = ops->start_cable_test_tdr(phydev, info->extack, &cfg);
@@ -365,7 +368,8 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info) 
		ethnl_cable_test_started(phydev,

					 ETHTOOL_MSG_CABLE_TEST_TDR_NTF);



out_rtnl:

out_unlock:

	netdev_unlock_ops(dev);

	rtnl_unlock();

out_dev_put:

	ethnl_parse_header_dev_put(&req_info);

net/ethtool/cmis_fw_update.c

+6 −1
Original line number Diff line number Diff line
@@ -418,8 +418,13 @@ cmis_fw_update_commit_image(struct ethtool_cmis_cdb *cdb, 
static int cmis_fw_update_reset(struct net_device *dev)

{

	__u32 reset_data = ETH_RESET_PHY;

	int ret;



	return dev->ethtool_ops->reset(dev, &reset_data);

	netdev_lock_ops(dev);

	ret = dev->ethtool_ops->reset(dev, &reset_data);

	netdev_unlock_ops(dev);



	return ret;

}



void

net/ethtool/features.c

+4 −2
Original line number Diff line number Diff line
@@ -234,9 +234,10 @@ int ethnl_set_features(struct sk_buff *skb, struct genl_info *info) 
	dev = req_info.dev;



	rtnl_lock();

	netdev_lock_ops(dev);

	ret = ethnl_ops_begin(dev);

	if (ret < 0)

		goto out_rtnl;

		goto out_unlock;

	ethnl_features_to_bitmap(old_active, dev->features);

	ethnl_features_to_bitmap(old_wanted, dev->wanted_features);

	ret = ethnl_parse_bitset(req_wanted, req_mask, NETDEV_FEATURE_COUNT,
@@ -286,7 +287,8 @@ int ethnl_set_features(struct sk_buff *skb, struct genl_info *info) 


out_ops:

	ethnl_ops_complete(dev);

out_rtnl:

out_unlock:

	netdev_unlock_ops(dev);

	rtnl_unlock();

	ethnl_parse_header_dev_put(&req_info);

	return ret;