net/sched: Restrict conditions for adding duplicating netems to qdisc tree

netem_enqueue's duplication prevention logic breaks when a netem
resides in a qdisc tree with other netems - this can lead to a
soft lockup and OOM loop in netem_dequeue, as seen in [1].
Ensure that a duplicating netem cannot exist in a tree with other
netems.

Previous approaches suggested in discussions in chronological order:

1) Track duplication status or ttl in the sk_buff struct. Considered
too specific a use case to extend such a struct, though this would
be a resilient fix and address other previous and potential future
DOS bugs like the one described in loopy fun [2].

2) Restrict netem_enqueue recursion depth like in act_mirred with a
per cpu variable. However, netem_dequeue can call enqueue on its
child, and the depth restriction could be bypassed if the child is a
netem.

3) Use the same approach as in 2, but add metadata in netem_skb_cb
to handle the netem_dequeue case and track a packet's involvement
in duplication. This is an overly complex approach, and Jamal
notes that the skb cb can be overwritten to circumvent this
safeguard.

4) Prevent the addition of a netem to a qdisc tree if its ancestral
path contains a netem. However, filters and actions can cause a
packet to change paths when re-enqueued to the root from netem
duplication, leading us to the current solution: prevent a
duplicating netem from inhabiting the same tree as other netems.

[1] https://lore.kernel.org/netdev/8DuRWwfqjoRDLDmBMlIfbrsZg9Gx50DHJc1ilxsEBNe2D6NMoigR_eIRIG0LOjMc3r10nUUZtArXx4oZBIdUfZQrwjcQhdinnMis_0G7VEk=@willsroot.io/
[2] https://lwn.net/Articles/719297/

Fixes: 0afb51e72855 ("[PKT_SCHED]: netem: reinsert for duplication")
Reported-by: William Liu <will@willsroot.io>
Reported-by: Savino Dicanosa <savy@syst3mfailure.io>
Signed-off-by: William Liu <will@willsroot.io>
Signed-off-by: Savino Dicanosa <savy@syst3mfailure.io>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Link: https://patch.msgid.link/20250708164141.875402-1-will@willsroot.io
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index fdd79d3ccd8ce76fbc7dc279b21898377a3bf8b8..eafc316ae319e3f8c23b0cb0c58fdf54be102213 100644 (file)
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -973,6 +973,41 @@ static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla,
        return 0;
 }
 
+static const struct Qdisc_class_ops netem_class_ops;
+
+static int check_netem_in_tree(struct Qdisc *sch, bool duplicates,
+                              struct netlink_ext_ack *extack)
+{
+       struct Qdisc *root, *q;
+       unsigned int i;
+
+       root = qdisc_root_sleeping(sch);
+
+       if (sch != root && root->ops->cl_ops == &netem_class_ops) {
+               if (duplicates ||
+                   ((struct netem_sched_data *)qdisc_priv(root))->duplicate)
+                       goto err;
+       }
+
+       if (!qdisc_dev(root))
+               return 0;
+
+       hash_for_each(qdisc_dev(root)->qdisc_hash, i, q, hash) {
+               if (sch != q && q->ops->cl_ops == &netem_class_ops) {
+                       if (duplicates ||
+                           ((struct netem_sched_data *)qdisc_priv(q))->duplicate)
+                               goto err;
+               }
+       }
+
+       return 0;
+
+err:
+       NL_SET_ERR_MSG(extack,
+                      "netem: cannot mix duplicating netems with other netems in tree");
+       return -EINVAL;
+}
+
 /* Parse netlink message to set options */
 static int netem_change(struct Qdisc *sch, struct nlattr *opt,
                        struct netlink_ext_ack *extack)
@@ -1031,6 +1066,11 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
        q->gap = qopt->gap;
        q->counter = 0;
        q->loss = qopt->loss;
+
+       ret = check_netem_in_tree(sch, qopt->duplicate, extack);
+       if (ret)
+               goto unlock;
+
        q->duplicate = qopt->duplicate;
 
        /* for compatibility with earlier versions.