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 <shemminger@vyatta.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 4cff44235773374b4cedc5f88cf9e8946e061341..c2e628dfaaccb0f2dacffa9bbc5dec34bbb5f291 100644 (file)
--- 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);