net_sched: drr: 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 drr, there won't be a UAF, but the code will add the same
classifier to the list twice, which will cause memory corruption.

In addition to checking for qlen being zero, this patch checks whether the
class was already added to the active_list (cl_is_active) before adding
to the list to cover for the reentrant case.

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

Fixes: 37d9cf1a3c ("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-2-victor@mojatatu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
pull/1194/merge
Victor Nogueira 2025-04-25 19:07:05 -03:00 committed by Jakub Kicinski
parent dfd76010f8
commit f99a3fbf02
1 changed files with 6 additions and 3 deletions

View File

@ -35,6 +35,11 @@ struct drr_sched {
struct Qdisc_class_hash clhash; struct Qdisc_class_hash clhash;
}; };
static bool cl_is_active(struct drr_class *cl)
{
return !list_empty(&cl->alist);
}
static struct drr_class *drr_find_class(struct Qdisc *sch, u32 classid) static struct drr_class *drr_find_class(struct Qdisc *sch, u32 classid)
{ {
struct drr_sched *q = qdisc_priv(sch); struct drr_sched *q = qdisc_priv(sch);
@ -337,7 +342,6 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct drr_sched *q = qdisc_priv(sch); struct drr_sched *q = qdisc_priv(sch);
struct drr_class *cl; struct drr_class *cl;
int err = 0; int err = 0;
bool first;
cl = drr_classify(skb, sch, &err); cl = drr_classify(skb, sch, &err);
if (cl == NULL) { if (cl == NULL) {
@ -347,7 +351,6 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
return err; return err;
} }
first = !cl->qdisc->q.qlen;
err = qdisc_enqueue(skb, cl->qdisc, to_free); err = qdisc_enqueue(skb, cl->qdisc, to_free);
if (unlikely(err != NET_XMIT_SUCCESS)) { if (unlikely(err != NET_XMIT_SUCCESS)) {
if (net_xmit_drop_count(err)) { if (net_xmit_drop_count(err)) {
@ -357,7 +360,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
return err; return err;
} }
if (first) { if (!cl_is_active(cl)) {
list_add_tail(&cl->alist, &q->active); list_add_tail(&cl->alist, &q->active);
cl->deficit = cl->quantum; cl->deficit = cl->quantum;
} }