[PATCH net v2] net/sched: sch_qfq: revalidate aggregate after peek

[Jérémy Jean]

A netem child can re-enter qfq_enqueue() through a duplicating netem.
If lmax was lowered with packets queued, qfq_change_agg() can move the
class and free its old aggregate. This can cause a UAF or put the
class on the wrong active list.

Restart qfq_dequeue() if a selection peek changes the aggregate. Charge
the serving aggregate before the post-dequeue peek. After this peek,
requeue only an active class using cl->agg. Recheck activity and use
cl->agg after the peek in qfq_enqueue().

Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
---
Changes in v2 after review from Sashiko:
- Cover the post-dequeue peek without moving service accounting.
- Clarify the trigger.

v1: https://lore.kernel.org/all/20261106200236.716641-2-Jeremy.Jean@xxxxxxxxxxxxxxxxx/
Sashiko: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106200236.716641-2-Jeremy.Jean%40oss.cyber.gouv.fr

 net/sched/sch_qfq.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index e900890..82903f9 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -992,8 +992,7 @@ static void qfq_update_eligible(struct qfq_sched *q)
 }
 
 /* Dequeue head packet of the head class in the DRR queue of the aggregate. */
-static struct sk_buff *agg_dequeue(struct qfq_aggregate *agg,
-				   struct qfq_class *cl, unsigned int len)
+static struct sk_buff *agg_dequeue(struct qfq_class *cl, unsigned int len)
 {
 	struct sk_buff *skb = qdisc_dequeue_peeked(cl->qdisc);
 
@@ -1002,13 +1001,6 @@ static struct sk_buff *agg_dequeue(struct qfq_aggregate *agg,
 
 	cl->deficit -= (int) len;
 
-	if (cl->qdisc->q.qlen == 0) /* no more packets, remove from list */
-		list_del_init(&cl->alist);
-	else if (cl->deficit < qdisc_peek_len(cl->qdisc)) {
-		cl->deficit += agg->lmax;
-		list_move_tail(&cl->alist, &agg->active);
-	}
-
 	return skb;
 }
 
@@ -1103,6 +1095,7 @@ static void qfq_schedule_agg(struct qfq_sched *q, struct qfq_aggregate *agg);
 static struct sk_buff *qfq_dequeue(struct Qdisc *sch)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
+restart:;
 	struct qfq_aggregate *in_serv_agg = q->in_serv_agg;
 	struct qfq_class *cl;
 	struct sk_buff *skb = NULL;
@@ -1112,8 +1105,11 @@ static struct sk_buff *qfq_dequeue(struct Qdisc *sch)
 	if (in_serv_agg == NULL)
 		return NULL;
 
-	if (!list_empty(&in_serv_agg->active))
+	if (!list_empty(&in_serv_agg->active)) {
 		skb = qfq_peek_skb(in_serv_agg, &cl, &len);
+		if (unlikely(in_serv_agg != q->in_serv_agg || cl->agg != in_serv_agg))
+			goto restart;
+	}
 
 	/*
 	 * If there are no active classes in the in-service aggregate,
@@ -1151,13 +1147,15 @@ static struct sk_buff *qfq_dequeue(struct Qdisc *sch)
 		 */
 		in_serv_agg = q->in_serv_agg = qfq_choose_next_agg(q);
 		skb = qfq_peek_skb(in_serv_agg, &cl, &len);
+		if (unlikely(in_serv_agg != q->in_serv_agg || cl->agg != in_serv_agg))
+			goto restart;
 	}
 	if (!skb)
 		return NULL;
 
 	qdisc_qlen_dec(sch);
 
-	skb = agg_dequeue(in_serv_agg, cl, len);
+	skb = agg_dequeue(cl, len);
 
 	if (!skb) {
 		qdisc_qlen_inc(sch);
@@ -1181,6 +1179,13 @@ static struct sk_buff *qfq_dequeue(struct Qdisc *sch)
 		 len, (unsigned long long) in_serv_agg->F,
 		 (unsigned long long) q->V);
 
+	if (cl->qdisc->q.qlen == 0) /* no more packets, remove from list */
+		list_del_init(&cl->alist);
+	else if (cl->deficit < qdisc_peek_len(cl->qdisc) && cl_is_active(cl)) {
+		cl->deficit += cl->agg->lmax;
+		list_move_tail(&cl->alist, &cl->agg->active);
+	}
+
 	return skb;
 }
 
@@ -1274,9 +1279,10 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	/* 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_is_active(cl) &&
+		    list_first_entry(&cl->agg->active, struct qfq_class, alist)
 		    == cl && cl->deficit < len)
-			list_move_tail(&cl->alist, &agg->active);
+			list_move_tail(&cl->alist, &cl->agg->active);
 
 		return err;
 	}
-- 
2.47.3