Commit f139f37d authored by Victor Nogueira's avatar Victor Nogueira Committed by Jakub Kicinski
Browse files

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: 37d9cf1a ("sched: Fix detection of empty queues in child qdiscs")
Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarVictor Nogueira <victor@mojatatu.com>
Link: https://patch.msgid.link/20250425220710.3964791-5-victor@mojatatu.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 1a6d0c00

net/sched/sch_qfq.c

+7 −4
Original line number Diff line number Diff line
@@ -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)