Commit 5e28d5a3 authored by Xiang Mei's avatar Xiang Mei Committed by David S. Miller
Browse files

net/sched: sch_qfq: Fix race condition on qfq_aggregate 



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: 462dbc91 ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
Signed-off-by: default avatarXiang Mei <xmei5@asu.edu>
Reviewed-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 7727ec15

net/sched/sch_qfq.c

+21 −9
Original line number Diff line number Diff line
@@ -412,7 +412,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;
@@ -443,12 +443,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)

	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) {

		NL_SET_ERR_MSG_FMT_MOD(extack,
@@ -555,10 +559,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;

}
@@ -625,6 +629,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;
@@ -633,8 +638,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);
@@ -651,8 +661,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(d, NULL, &cl->bstats, true) < 0 ||

	    gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||