[net PATCH v2] net: sched: lock imbalance in hhf qdisc

[net PATCH v2] net: sched: lock imbalance in hhf qdisc

[John Fastabend]

hhf_change() takes the sch_tree_lock and releases it but misses the
error cases. Fix the missed case here.

To reproduce try a command like this,

# tc qdisc change dev p3p2 root hhf quantum 40960 non_hh_weight 300000

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 net/sched/sch_hhf.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index edee03d..6e957c3 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -553,11 +553,6 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
 	if (err < 0)
 		return err;
 
-	sch_tree_lock(sch);
-
-	if (tb[TCA_HHF_BACKLOG_LIMIT])
-		sch->limit = nla_get_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
-
 	if (tb[TCA_HHF_QUANTUM])
 		new_quantum = nla_get_u32(tb[TCA_HHF_QUANTUM]);
 
@@ -567,6 +562,12 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
 	non_hh_quantum = (u64)new_quantum * new_hhf_non_hh_weight;
 	if (non_hh_quantum > INT_MAX)
 		return -EINVAL;
+
+	sch_tree_lock(sch);
+
+	if (tb[TCA_HHF_BACKLOG_LIMIT])
+		sch->limit = nla_get_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
+
 	q->quantum = new_quantum;
 	q->hhf_non_hh_weight = new_hhf_non_hh_weight;
 

Re: [net PATCH v2] net: sched: lock imbalance in hhf qdisc

[Eric Dumazet]

On Thu, 2014-05-01 at 09:23 -0700, John Fastabend wrote:
> hhf_change() takes the sch_tree_lock and releases it but misses the
> error cases. Fix the missed case here.
> 
> To reproduce try a command like this,
> 
> # tc qdisc change dev p3p2 root hhf quantum 40960 non_hh_weight 300000
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>  net/sched/sch_hhf.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

Signed-off-by: Eric Dumazet <edumazet@google.com>

Re: [net PATCH v2] net: sched: lock imbalance in hhf qdisc

[Terry Lam]

Thanks a lot for the fix, John and Eric!

Regards,
Terry

On Thu, May 1, 2014 at 9:23 AM, John Fastabend <john.fastabend@gmail.com> wrote:
> hhf_change() takes the sch_tree_lock and releases it but misses the
> error cases. Fix the missed case here.
>
> To reproduce try a command like this,
>
> # tc qdisc change dev p3p2 root hhf quantum 40960 non_hh_weight 300000
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>  net/sched/sch_hhf.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
> index edee03d..6e957c3 100644
> --- a/net/sched/sch_hhf.c
> +++ b/net/sched/sch_hhf.c
> @@ -553,11 +553,6 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
>         if (err < 0)
>                 return err;
>
> -       sch_tree_lock(sch);
> -
> -       if (tb[TCA_HHF_BACKLOG_LIMIT])
> -               sch->limit = nla_get_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
> -
>         if (tb[TCA_HHF_QUANTUM])
>                 new_quantum = nla_get_u32(tb[TCA_HHF_QUANTUM]);
>
> @@ -567,6 +562,12 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
>         non_hh_quantum = (u64)new_quantum * new_hhf_non_hh_weight;
>         if (non_hh_quantum > INT_MAX)
>                 return -EINVAL;
> +
> +       sch_tree_lock(sch);
> +
> +       if (tb[TCA_HHF_BACKLOG_LIMIT])
> +               sch->limit = nla_get_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
> +
>         q->quantum = new_quantum;
>         q->hhf_non_hh_weight = new_hhf_non_hh_weight;
>
>

Re: [net PATCH v2] net: sched: lock imbalance in hhf qdisc

[David Miller]

From: John Fastabend <john.fastabend@gmail.com>
Date: Thu, 01 May 2014 09:23:06 -0700

> hhf_change() takes the sch_tree_lock and releases it but misses the
> error cases. Fix the missed case here.
> 
> To reproduce try a command like this,
> 
> # tc qdisc change dev p3p2 root hhf quantum 40960 non_hh_weight 300000
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

Applied and queued up for -stable, thanks.