Skip to contentSkip to conversation

net: sched: lock imbalance in hhf qdisc

PATCH In review John Fastabend 4 participants 4 messages May 1, 2014 – May 4, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 net: sched: lock imbalance in hhf qdisc

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
---
 net/sched/sch_hhf.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
net/sched/sch_hhf.c +6 -5
index edee03d..6e957c3 100644
@@ -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;
 
#1 lore raw
Quoted 11 lines · 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
---
 net/sched/sch_hhf.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
#2 lore raw
In reply to John Fastabend

Thanks a lot for the fix, John and Eric!

Regards,
Terry

Quoted 42 lines · 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
---
 net/sched/sch_hhf.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
net/sched/sch_hhf.c +6 -5
index edee03d..6e957c3 100644
@@ -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;