net/sched: sch_qfq: Fix race condition on qfq_aggregate

[ Upstream commit 5e28d5a3f774f118896aec17a3a20a9c5c9dfc64 ]

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: 462dbc9101ac ("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>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 1ee15db5fcc8c98e71ab68253bb3260a9e3115a8..e412340f639d2f942c89b4f73f3b250d943c3d45 100644 (file)
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -414,7 +414,7 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
        bool existing = false;
        struct nlattr *tb[TCA_QFQ_MAX + 1];
        struct qfq_aggregate *new_agg = NULL;
-       u32 weight, lmax, inv_w;
+       u32 weight, lmax, inv_w, old_weight, old_lmax;
        int err;
        int delta_w;
 
@@ -448,12 +448,16 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
        inv_w = ONE_FP / weight;
        weight = ONE_FP / inv_w;
 
-       if (cl != NULL &&
-           lmax == cl->agg->lmax &&
-           weight == cl->agg->class_weight)
-               return 0; /* nothing to change */
+       if (cl != NULL) {
+               sch_tree_lock(sch);
+               old_weight = cl->agg->class_weight;
+               old_lmax   = cl->agg->lmax;
+               sch_tree_unlock(sch);
+               if (lmax == old_lmax && weight == old_weight)
+                       return 0; /* nothing to change */
+       }
 
-       delta_w = weight - (cl ? cl->agg->class_weight : 0);
+       delta_w = weight - (cl ? old_weight : 0);
 
        if (q->wsum + delta_w > QFQ_MAX_WSUM) {
                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)
 
        qdisc_purge_queue(cl->qdisc);
        qdisc_class_hash_remove(&q->clhash, &cl->common);
+       qfq_destroy_class(sch, cl);
 
        sch_tree_unlock(sch);
 
-       qfq_destroy_class(sch, cl);
        return 0;
 }
 
@@ -624,6 +628,7 @@ static int qfq_dump_class(struct Qdisc *sch, unsigned long arg,
 {
        struct qfq_class *cl = (struct qfq_class *)arg;
        struct nlattr *nest;
+       u32 class_weight, lmax;
 
        tcm->tcm_parent = TC_H_ROOT;
        tcm->tcm_handle = cl->common.classid;
@@ -632,8 +637,13 @@ static int qfq_dump_class(struct Qdisc *sch, unsigned long arg,
        nest = nla_nest_start_noflag(skb, TCA_OPTIONS);
        if (nest == NULL)
                goto nla_put_failure;
-       if (nla_put_u32(skb, TCA_QFQ_WEIGHT, cl->agg->class_weight) ||
-           nla_put_u32(skb, TCA_QFQ_LMAX, cl->agg->lmax))
+
+       sch_tree_lock(sch);
+       class_weight    = cl->agg->class_weight;
+       lmax            = cl->agg->lmax;
+       sch_tree_unlock(sch);
+       if (nla_put_u32(skb, TCA_QFQ_WEIGHT, class_weight) ||
+           nla_put_u32(skb, TCA_QFQ_LMAX, lmax))
                goto nla_put_failure;
        return nla_nest_end(skb, nest);
 
@@ -650,8 +660,10 @@ static int qfq_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 
        memset(&xstats, 0, sizeof(xstats));
 
+       sch_tree_lock(sch);
        xstats.weight = cl->agg->class_weight;
        xstats.lmax = cl->agg->lmax;
+       sch_tree_unlock(sch);
 
        if (gnet_stats_copy_basic(qdisc_root_sleeping_running(sch),
                                  d, NULL, &cl->bstats) < 0 ||