From 119b3d386985fcd477b3131190c041516a73f83a 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 817fb15dfd988d8dda916ee04fa506f0c466b9d6 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 4cff44235773..c2e628dfaacc 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