From 1922acf04cd57dcce9080c117e4df806370ce4eb Mon Sep 17 00:00:00 2001 From: stephen hemminger Date: Wed, 2 Feb 2011 15:19:51 +0000 Subject: [PATCH] sfq: deadlock in error path The change to allow divisor to be a parameter (in 2.6.38-rc1) commit 218094e828bf981aebe14eacf2bc96dee996521c introduced a possible deadlock caught by sparse. The scheduler tree lock was left locked in the case of an incorrect divisor value. Simplest fix is to move test outside of lock which also solves problem of partial update. Signed-off-by: Stephen Hemminger Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/sched/sch_sfq.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c index 4cff442357733..c2e628dfaaccb 100644 --- a/net/sched/sch_sfq.c +++ b/net/sched/sch_sfq.c @@ -491,17 +491,18 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt) if (opt->nla_len < nla_attr_size(sizeof(*ctl))) return -EINVAL; + if (ctl->divisor && + (!is_power_of_2(ctl->divisor) || ctl->divisor > 65536)) + return -EINVAL; + sch_tree_lock(sch); q->quantum = ctl->quantum ? : psched_mtu(qdisc_dev(sch)); q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum); q->perturb_period = ctl->perturb_period * HZ; if (ctl->limit) q->limit = min_t(u32, ctl->limit, SFQ_DEPTH - 1); - if (ctl->divisor) { - if (!is_power_of_2(ctl->divisor) || ctl->divisor > 65536) - return -EINVAL; + if (ctl->divisor) q->divisor = ctl->divisor; - } qlen = sch->q.qlen; while (sch->q.qlen > q->limit) sfq_drop(sch); -- 2.39.5