mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2025-08-15 18:15:09 +00:00
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: 462dbc9101
("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>
This commit is contained in:
parent
7727ec1523
commit
5e28d5a3f7
@ -412,7 +412,7 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
|
|||||||
bool existing = false;
|
bool existing = false;
|
||||||
struct nlattr *tb[TCA_QFQ_MAX + 1];
|
struct nlattr *tb[TCA_QFQ_MAX + 1];
|
||||||
struct qfq_aggregate *new_agg = NULL;
|
struct qfq_aggregate *new_agg = NULL;
|
||||||
u32 weight, lmax, inv_w;
|
u32 weight, lmax, inv_w, old_weight, old_lmax;
|
||||||
int err;
|
int err;
|
||||||
int delta_w;
|
int delta_w;
|
||||||
|
|
||||||
@ -443,12 +443,16 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
|
|||||||
inv_w = ONE_FP / weight;
|
inv_w = ONE_FP / weight;
|
||||||
weight = ONE_FP / inv_w;
|
weight = ONE_FP / inv_w;
|
||||||
|
|
||||||
if (cl != NULL &&
|
if (cl != NULL) {
|
||||||
lmax == cl->agg->lmax &&
|
sch_tree_lock(sch);
|
||||||
weight == cl->agg->class_weight)
|
old_weight = cl->agg->class_weight;
|
||||||
return 0; /* nothing to change */
|
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) {
|
if (q->wsum + delta_w > QFQ_MAX_WSUM) {
|
||||||
NL_SET_ERR_MSG_FMT_MOD(extack,
|
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_purge_queue(cl->qdisc);
|
||||||
qdisc_class_hash_remove(&q->clhash, &cl->common);
|
qdisc_class_hash_remove(&q->clhash, &cl->common);
|
||||||
|
qfq_destroy_class(sch, cl);
|
||||||
|
|
||||||
sch_tree_unlock(sch);
|
sch_tree_unlock(sch);
|
||||||
|
|
||||||
qfq_destroy_class(sch, cl);
|
|
||||||
return 0;
|
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 qfq_class *cl = (struct qfq_class *)arg;
|
||||||
struct nlattr *nest;
|
struct nlattr *nest;
|
||||||
|
u32 class_weight, lmax;
|
||||||
|
|
||||||
tcm->tcm_parent = TC_H_ROOT;
|
tcm->tcm_parent = TC_H_ROOT;
|
||||||
tcm->tcm_handle = cl->common.classid;
|
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);
|
nest = nla_nest_start_noflag(skb, TCA_OPTIONS);
|
||||||
if (nest == NULL)
|
if (nest == NULL)
|
||||||
goto nla_put_failure;
|
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;
|
goto nla_put_failure;
|
||||||
return nla_nest_end(skb, nest);
|
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));
|
memset(&xstats, 0, sizeof(xstats));
|
||||||
|
|
||||||
|
sch_tree_lock(sch);
|
||||||
xstats.weight = cl->agg->class_weight;
|
xstats.weight = cl->agg->class_weight;
|
||||||
xstats.lmax = cl->agg->lmax;
|
xstats.lmax = cl->agg->lmax;
|
||||||
|
sch_tree_unlock(sch);
|
||||||
|
|
||||||
if (gnet_stats_copy_basic(d, NULL, &cl->bstats, true) < 0 ||
|
if (gnet_stats_copy_basic(d, NULL, &cl->bstats, true) < 0 ||
|
||||||
gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
|
gnet_stats_copy_rate_est(d, &cl->rate_est) < 0 ||
|
||||||
|
Loading…
Reference in New Issue
Block a user