Skip to content

Commit c1bf840

Browse files
committed
net/sched: sch_qfq: Fix race condition on qfq_aggregate
jira VULN-89290 jira VULN-89289 cve CVE-2025-38477 commit-author Xiang Mei <xmei5@asu.edu> commit 5e28d5a A race condition can occur when 'agg' is modified in qfq_change_agg (called during qfq_enqueue) while other threads access it concurrently. For example, qfq_dump_class may trigger a NULL dereference, and qfq_delete_class may cause a use-after-free. This patch addresses the issue by: 1. Moved qfq_destroy_class into the critical section. 2. Added sch_tree_lock protection to qfq_dump_class and qfq_dump_class_stats. Fixes: 462dbc9 ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost") Signed-off-by: Xiang Mei <xmei5@asu.edu> Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> (cherry picked from commit 5e28d5a) Signed-off-by: Jonathan Maple <jmaple@ciq.com>
1 parent 325a6c5 commit c1bf840

File tree

1 file changed

+21
-9
lines changed

1 file changed

+21
-9
lines changed

net/sched/sch_qfq.c

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
412412
bool existing = false;
413413
struct nlattr *tb[TCA_QFQ_MAX + 1];
414414
struct qfq_aggregate *new_agg = NULL;
415-
u32 weight, lmax, inv_w;
415+
u32 weight, lmax, inv_w, old_weight, old_lmax;
416416
int err;
417417
int delta_w;
418418

@@ -446,12 +446,16 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
446446
inv_w = ONE_FP / weight;
447447
weight = ONE_FP / inv_w;
448448

449-
if (cl != NULL &&
450-
lmax == cl->agg->lmax &&
451-
weight == cl->agg->class_weight)
452-
return 0; /* nothing to change */
449+
if (cl != NULL) {
450+
sch_tree_lock(sch);
451+
old_weight = cl->agg->class_weight;
452+
old_lmax = cl->agg->lmax;
453+
sch_tree_unlock(sch);
454+
if (lmax == old_lmax && weight == old_weight)
455+
return 0; /* nothing to change */
456+
}
453457

454-
delta_w = weight - (cl ? cl->agg->class_weight : 0);
458+
delta_w = weight - (cl ? old_weight : 0);
455459

456460
if (q->wsum + delta_w > QFQ_MAX_WSUM) {
457461
pr_notice("qfq: total weight out of range (%d + %u)\n",
@@ -554,10 +558,10 @@ static int qfq_delete_class(struct Qdisc *sch, unsigned long arg,
554558

555559
qdisc_purge_queue(cl->qdisc);
556560
qdisc_class_hash_remove(&q->clhash, &cl->common);
561+
qfq_destroy_class(sch, cl);
557562

558563
sch_tree_unlock(sch);
559564

560-
qfq_destroy_class(sch, cl);
561565
return 0;
562566
}
563567

@@ -624,6 +628,7 @@ static int qfq_dump_class(struct Qdisc *sch, unsigned long arg,
624628
{
625629
struct qfq_class *cl = (struct qfq_class *)arg;
626630
struct nlattr *nest;
631+
u32 class_weight, lmax;
627632

628633
tcm->tcm_parent = TC_H_ROOT;
629634
tcm->tcm_handle = cl->common.classid;
@@ -632,8 +637,13 @@ static int qfq_dump_class(struct Qdisc *sch, unsigned long arg,
632637
nest = nla_nest_start_noflag(skb, TCA_OPTIONS);
633638
if (nest == NULL)
634639
goto nla_put_failure;
635-
if (nla_put_u32(skb, TCA_QFQ_WEIGHT, cl->agg->class_weight) ||
636-
nla_put_u32(skb, TCA_QFQ_LMAX, cl->agg->lmax))
640+
641+
sch_tree_lock(sch);
642+
class_weight = cl->agg->class_weight;
643+
lmax = cl->agg->lmax;
644+
sch_tree_unlock(sch);
645+
if (nla_put_u32(skb, TCA_QFQ_WEIGHT, class_weight) ||
646+
nla_put_u32(skb, TCA_QFQ_LMAX, lmax))
637647
goto nla_put_failure;
638648
return nla_nest_end(skb, nest);
639649

@@ -650,8 +660,10 @@ static int qfq_dump_class_stats(struct Qdisc *sch, unsigned long arg,
650660

651661
memset(&xstats, 0, sizeof(xstats));
652662

663+
sch_tree_lock(sch);
653664
xstats.weight = cl->agg->class_weight;
654665
xstats.lmax = cl->agg->lmax;
666+
sch_tree_unlock(sch);
655667

656668
if (gnet_stats_copy_basic(d, NULL, &cl->bstats, true) < 0 ||
657669
gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||

0 commit comments

Comments
 (0)