@@ -1268,38 +1268,8 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
struct qdisc_size_table *stab;
ops = qdisc_lookup_ops(kind);
-#ifdef CONFIG_MODULES
- if (ops == NULL && kind != NULL) {
- char name[IFNAMSIZ];
- if (nla_strscpy(name, kind, IFNAMSIZ) >= 0) {
- /* We dropped the RTNL semaphore in order to
- * perform the module load. So, even if we
- * succeeded in loading the module we have to
- * tell the caller to replay the request. We
- * indicate this using -EAGAIN.
- * We replay the request because the device may
- * go away in the mean time.
- */
- netdev_unlock_ops(dev);
- rtnl_unlock();
- request_module(NET_SCH_ALIAS_PREFIX "%s", name);
- rtnl_lock();
- netdev_lock_ops(dev);
- ops = qdisc_lookup_ops(kind);
- if (ops != NULL) {
- /* We will try again qdisc_lookup_ops,
- * so don't keep a reference.
- */
- module_put(ops->owner);
- err = -EAGAIN;
- goto err_out;
- }
- }
- }
-#endif
-
- err = -ENOENT;
if (!ops) {
+ err = -ENOENT;
NL_SET_ERR_MSG(extack, "Specified qdisc kind is unknown");
goto err_out;
}
@@ -1624,8 +1594,7 @@ static int __tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
struct netlink_ext_ack *extack,
struct net_device *dev,
struct nlattr *tca[TCA_MAX + 1],
- struct tcmsg *tcm,
- bool *replay)
+ struct tcmsg *tcm)
{
struct Qdisc *q = NULL;
struct Qdisc *p = NULL;
@@ -1790,13 +1759,8 @@ static int __tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
tcm->tcm_parent, tcm->tcm_handle,
tca, &err, extack);
}
- if (q == NULL) {
- if (err == -EAGAIN) {
- *replay = true;
- return 0;
- }
+ if (!q)
return err;
- }
graft:
err = qdisc_graft(dev, p, skb, n, clid, q, NULL, extack);
@@ -1809,6 +1773,27 @@ static int __tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
return 0;
}
+static void request_qdisc_module(struct nlattr *kind)
+{
+ struct Qdisc_ops *ops;
+ char name[IFNAMSIZ];
+
+ if (!kind)
+ return;
+
+ ops = qdisc_lookup_ops(kind);
+ if (ops) {
+ module_put(ops->owner);
+ return;
+ }
+
+ if (nla_strscpy(name, kind, IFNAMSIZ) >= 0) {
+ rtnl_unlock();
+ request_module(NET_SCH_ALIAS_PREFIX "%s", name);
+ rtnl_lock();
+ }
+}
+
/*
* Create/change qdisc.
*/
@@ -1819,27 +1804,23 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
struct nlattr *tca[TCA_MAX + 1];
struct net_device *dev;
struct tcmsg *tcm;
- bool replay;
int err;
-replay:
- /* Reinit, just in case something touches this. */
err = nlmsg_parse_deprecated(n, sizeof(*tcm), tca, TCA_MAX,
rtm_tca_policy, extack);
if (err < 0)
return err;
+ request_qdisc_module(tca[TCA_KIND]);
+
tcm = nlmsg_data(n);
dev = __dev_get_by_index(net, tcm->tcm_ifindex);
if (!dev)
return -ENODEV;
- replay = false;
netdev_lock_ops(dev);
- err = __tc_modify_qdisc(skb, n, extack, dev, tca, tcm, &replay);
+ err = __tc_modify_qdisc(skb, n, extack, dev, tca, tcm);
netdev_unlock_ops(dev);
- if (replay)
- goto replay;
return err;
}
Eric reports that by the time we call netdev_lock_ops after rtnl_unlock/rtnl_lock, the dev might point to an invalid device. As suggested by Jakub in [0], move rtnl lock/unlock and request_module outside of qdisc_create. This removes extra complexity with relocking the netdev. 0: https://lore.kernel.org/netdev/20250325032803.1542c15e@kernel.org/ Fixes: a0527ee2df3f ("net: hold netdev instance lock during qdisc ndo_setup_tc") Reported-by: Eric Dumazet <edumazet@google.com> Link: https://lore.kernel.org/netdev/20250305163732.2766420-1-sdf@fomichev.me/T/#me8dfd778ea4c4463acab55644e3f9836bc608771 Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> --- net/sched/sch_api.c | 73 +++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 46 deletions(-)