nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

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

Browse source 
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: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: Saeed Mahameed <saeed@kernel.org>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250305163732.2766420-11-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2025-03-05 08:37:28 -08:00
parent 97246d6d21
commit 2bcf4772e4
11 changed files with 82 additions and 26 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

2
drivers/net/netdevsim/ethtool.c
View file

@ -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;

16
net/dsa/conduit.c
View file

@ -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;

20
net/ethtool/cabletest.c
View file

@ -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);

7
net/ethtool/cmis_fw_update.c
View file

@ -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

6
net/ethtool/features.c
View file

@ -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;

6
net/ethtool/ioctl.c
View file

@ -2317,6 +2317,7 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
*/
busy = true;
netdev_hold(dev, &dev_tracker, GFP_KERNEL);
netdev_unlock_ops(dev);
rtnl_unlock();
if (rc == 0) {
@ -2331,8 +2332,10 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
do {
rtnl_lock();
netdev_lock_ops(dev);
rc = ops->set_phys_id(dev,
(i++ & 1) ? ETHTOOL_ID_OFF : ETHTOOL_ID_ON);
netdev_unlock_ops(dev);
rtnl_unlock();
if (rc)
break;
@ -2341,6 +2344,7 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
}
rtnl_lock();
netdev_lock_ops(dev);
netdev_put(dev, &dev_tracker);
busy = false;
@ -3140,6 +3144,7 @@ __dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr,
return -EPERM;
}
netdev_lock_ops(dev);
if (dev->dev.parent)
pm_runtime_get_sync(dev->dev.parent);
@ -3373,6 +3378,7 @@ __dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr,
out:
if (dev->dev.parent)
pm_runtime_put(dev->dev.parent);
netdev_unlock_ops(dev);
return rc;
}

8
net/ethtool/module.c
View file

@ -419,19 +419,21 @@ int ethnl_act_module_fw_flash(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;
ret = ethnl_module_fw_flash_validate(dev, info->extack);
if (ret < 0)
goto out_rtnl;
goto out_unlock;
ret = module_flash_fw(dev, tb, skb, info);
ethnl_ops_complete(dev);
out_rtnl:
out_unlock:
netdev_unlock_ops(dev);
rtnl_unlock();
ethnl_parse_header_dev_put(&req_info);
return ret;

12
net/ethtool/netlink.c
View file

@ -90,6 +90,8 @@ int ethnl_ops_begin(struct net_device *dev)
if (dev->dev.parent)
pm_runtime_get_sync(dev->dev.parent);
netdev_ops_assert_locked(dev);
if (!netif_device_present(dev) ||
dev->reg_state >= NETREG_UNREGISTERING) {
ret = -ENODEV;
@ -490,7 +492,11 @@ static int ethnl_default_doit(struct sk_buff *skb, struct genl_info *info)
ethnl_init_reply_data(reply_data, ops, req_info->dev);
rtnl_lock();
if (req_info->dev)
netdev_lock_ops(req_info->dev);
ret = ops->prepare_data(req_info, reply_data, info);
if (req_info->dev)
netdev_unlock_ops(req_info->dev);
rtnl_unlock();
if (ret < 0)
goto err_cleanup;
@ -548,7 +554,9 @@ static int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev,
ethnl_init_reply_data(ctx->reply_data, ctx->ops, dev);
rtnl_lock();
netdev_lock_ops(ctx->req_info->dev);
ret = ctx->ops->prepare_data(ctx->req_info, ctx->reply_data, info);
netdev_unlock_ops(ctx->req_info->dev);
rtnl_unlock();
if (ret < 0)
goto out;
@ -693,6 +701,7 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
dev = req_info.dev;
rtnl_lock();
netdev_lock_ops(dev);
dev->cfg_pending = kmemdup(dev->cfg, sizeof(*dev->cfg),
GFP_KERNEL_ACCOUNT);
if (!dev->cfg_pending) {
@ -720,6 +729,7 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info)
kfree(dev->cfg_pending);
out_tie_cfg:
dev->cfg_pending = dev->cfg;
netdev_unlock_ops(dev);
rtnl_unlock();
out_dev:
ethnl_parse_header_dev_put(&req_info);
@ -777,6 +787,8 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd,
req_info->dev = dev;
req_info->flags |= ETHTOOL_FLAG_COMPACT_BITSETS;
netdev_ops_assert_locked(dev);
ethnl_init_reply_data(reply_data, ops, dev);
ret = ops->prepare_data(req_info, reply_data, &info);
if (ret < 0)

20
net/ethtool/phy.c
View file

@ -158,18 +158,19 @@ int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info)
return ret;
rtnl_lock();
netdev_lock_ops(req_info.base.dev);
ret = ethnl_phy_parse_request(&req_info.base, tb, info->extack);
if (ret < 0)
goto err_unlock_rtnl;
goto err_unlock;
/* No PHY, return early */
if (!req_info.pdn)
goto err_unlock_rtnl;
goto err_unlock;
ret = ethnl_phy_reply_size(&req_info.base, info->extack);
if (ret < 0)
goto err_unlock_rtnl;
goto err_unlock;
reply_len = ret + ethnl_reply_header_size();
rskb = ethnl_reply_init(reply_len, req_info.base.dev,
@ -178,13 +179,14 @@ int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info)
info, &reply_payload);
if (!rskb) {
ret = -ENOMEM;
goto err_unlock_rtnl;
goto err_unlock;
}
ret = ethnl_phy_fill_reply(&req_info.base, rskb);
if (ret)
goto err_free_msg;
netdev_unlock_ops(req_info.base.dev);
rtnl_unlock();
ethnl_parse_header_dev_put(&req_info.base);
genlmsg_end(rskb, reply_payload);
@ -193,7 +195,8 @@ int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info)
err_free_msg:
nlmsg_free(rskb);
err_unlock_rtnl:
err_unlock:
netdev_unlock_ops(req_info.base.dev);
rtnl_unlock();
ethnl_parse_header_dev_put(&req_info.base);
return ret;
@ -290,10 +293,15 @@ int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
rtnl_lock();
if (ctx->phy_req_info->base.dev) {
ret = ethnl_phy_dump_one_dev(skb, ctx->phy_req_info->base.dev, cb);
dev = ctx->phy_req_info->base.dev;
netdev_lock_ops(dev);
ret = ethnl_phy_dump_one_dev(skb, dev, cb);
netdev_unlock_ops(dev);
} else {
for_each_netdev_dump(net, dev, ctx->ifindex) {
netdev_lock_ops(dev);
ret = ethnl_phy_dump_one_dev(skb, dev, cb);
netdev_unlock_ops(dev);
if (ret)
break;

2
net/ethtool/rss.c
View file

@ -345,7 +345,9 @@ int ethnl_rss_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
if (ctx->match_ifindex && ctx->match_ifindex != ctx->ifindex)
break;
netdev_lock_ops(dev);
ret = rss_dump_one_dev(skb, cb, dev);
netdev_unlock_ops(dev);
if (ret)
break;
}

9
net/ethtool/tsinfo.c
View file

@ -448,12 +448,15 @@ int ethnl_tsinfo_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
rtnl_lock();
if (ctx->req_info->base.dev) {
ret = ethnl_tsinfo_dump_one_net_topo(skb,
ctx->req_info->base.dev,
cb);
dev = ctx->req_info->base.dev;
netdev_lock_ops(dev);
ret = ethnl_tsinfo_dump_one_net_topo(skb, dev, cb);
netdev_unlock_ops(dev);
} else {
for_each_netdev_dump(net, dev, ctx->pos_ifindex) {
netdev_lock_ops(dev);
ret = ethnl_tsinfo_dump_one_net_topo(skb, dev, cb);
netdev_unlock_ops(dev);
if (ret < 0 && ret != -EOPNOTSUPP)
break;
ctx->pos_phyindex = 0;