net_sched: qfq: Fix double list add in class with netem as child qdisc

As described in Gerrard's report [1], there are use cases where a netem
child qdisc will make the parent qdisc's enqueue callback reentrant.
In the case of qfq, there won't be a UAF, but the code will add the same
classifier to the list twice, which will cause memory corruption.

This patch checks whether the class was already added to the agg->active
list (cl_is_active) before doing the addition to cater for the reentrant
case.

[1] https://lore.kernel.org/netdev/CAHcdcOm+03OD2j6R0=YHKqmy=VgJ8xEOKuP6c7mSgnp-TEJJbw@mail.gmail.com/

Fixes: 37d9cf1a3ce3 ("sched: Fix detection of empty queues in child qdiscs")
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Link: https://patch.msgid.link/20250425220710.3964791-5-victor@mojatatu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 687a932eb9b2f6e12b0dec49234323a22e014881..bf1282cb22ebae3a4a8c6fdd4aa3a5773e5593e2 100644 (file)
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -202,6 +202,11 @@ struct qfq_sched {
  */
 enum update_reason {enqueue, requeue};
 
+static bool cl_is_active(struct qfq_class *cl)
+{
+       return !list_empty(&cl->alist);
+}
+
 static struct qfq_class *qfq_find_class(struct Qdisc *sch, u32 classid)
 {
        struct qfq_sched *q = qdisc_priv(sch);
@@ -1215,7 +1220,6 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
        struct qfq_class *cl;
        struct qfq_aggregate *agg;
        int err = 0;
-       bool first;
 
        cl = qfq_classify(skb, sch, &err);
        if (cl == NULL) {
@@ -1237,7 +1241,6 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
        }
 
        gso_segs = skb_is_gso(skb) ? skb_shinfo(skb)->gso_segs : 1;
-       first = !cl->qdisc->q.qlen;
        err = qdisc_enqueue(skb, cl->qdisc, to_free);
        if (unlikely(err != NET_XMIT_SUCCESS)) {
                pr_debug("qfq_enqueue: enqueue failed %d\n", err);
@@ -1253,8 +1256,8 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
        ++sch->q.qlen;
 
        agg = cl->agg;
-       /* if the queue was not empty, then done here */
-       if (!first) {
+       /* if the class is active, then done here */
+       if (cl_is_active(cl)) {
                if (unlikely(skb == cl->qdisc->ops->peek(cl->qdisc)) &&
                    list_first_entry(&agg->active, struct qfq_class, alist)
                    == cl && cl->deficit < len)