From 3ba08b00e0d8413d79be9cab8ec085ceb6ae6fd6 Mon Sep 17 00:00:00 2001 From: Jarek Poplawski Date: Sat, 3 May 2008 20:46:29 -0700 Subject: sch_htb: remove from event queue in htb_parent_to_leaf() There is lack of removing a class from the event queue while changing from parent to leaf which can cause corruption of this rb tree. This patch fixes a bug introduced by my patch: "sch_htb: turn intermediate classes into leaves" commit: 160d5e10f87b1dc88fd9b84b31b1718e0fd76398. Many thanks to Jan 'yanek' Bortl for finding a way to reproduce this rare bug and narrowing the test case, which made possible proper diagnosing. This patch is recommended for all kernels starting from 2.6.20. Reported-and-tested-by: Jan 'yanek' Bortl Signed-off-by: Jarek Poplawski Signed-off-by: David S. Miller --- net/sched/sch_htb.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'net/sched') diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 66148cc4759..5bc1ed49018 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -1197,12 +1197,16 @@ static inline int htb_parent_last_child(struct htb_class *cl) return 1; } -static void htb_parent_to_leaf(struct htb_class *cl, struct Qdisc *new_q) +static void htb_parent_to_leaf(struct htb_sched *q, struct htb_class *cl, + struct Qdisc *new_q) { struct htb_class *parent = cl->parent; BUG_TRAP(!cl->level && cl->un.leaf.q && !cl->prio_activity); + if (parent->cmode != HTB_CAN_SEND) + htb_safe_rb_erase(&parent->pq_node, q->wait_pq + parent->level); + parent->level = 0; memset(&parent->un.inner, 0, sizeof(parent->un.inner)); INIT_LIST_HEAD(&parent->un.leaf.drop_list); @@ -1300,7 +1304,7 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg) htb_deactivate(q, cl); if (last_child) - htb_parent_to_leaf(cl, new_q); + htb_parent_to_leaf(q, cl, new_q); if (--cl->refcnt == 0) htb_destroy_class(sch, cl); -- cgit v1.2.3 From fa1b1cff3d06550d23ef540c4f97ca83c021b473 Mon Sep 17 00:00:00 2001 From: Jamal Hadi Salim Date: Mon, 5 May 2008 00:22:35 -0700 Subject: net_cls_act: Make act_simple use of netlink policy. Convert to netlink helpers by using netlink policy validation. As a side effect fixes a leak. Signed-off-by: Jamal Hadi Salim Signed-off-by: David S. Miller --- net/sched/act_simple.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) (limited to 'net/sched') diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c index 64b2d136c78..269ab51cd9b 100644 --- a/net/sched/act_simple.c +++ b/net/sched/act_simple.c @@ -6,7 +6,7 @@ * as published by the Free Software Foundation; either version * 2 of the License, or (at your option) any later version. * - * Authors: Jamal Hadi Salim (2005) + * Authors: Jamal Hadi Salim (2005-8) * */ @@ -34,6 +34,7 @@ static struct tcf_hashinfo simp_hash_info = { .lock = &simp_lock, }; +#define SIMP_MAX_DATA 32 static int tcf_simp(struct sk_buff *skb, struct tc_action *a, struct tcf_result *res) { struct tcf_defact *d = a->priv; @@ -69,23 +70,24 @@ static int tcf_simp_release(struct tcf_defact *d, int bind) return ret; } -static int alloc_defdata(struct tcf_defact *d, u32 datalen, void *defdata) +static int alloc_defdata(struct tcf_defact *d, char *defdata) { - d->tcfd_defdata = kmemdup(defdata, datalen, GFP_KERNEL); + d->tcfd_defdata = kstrndup(defdata, SIMP_MAX_DATA, GFP_KERNEL); if (unlikely(!d->tcfd_defdata)) return -ENOMEM; - d->tcfd_datalen = datalen; + return 0; } -static int realloc_defdata(struct tcf_defact *d, u32 datalen, void *defdata) +static int realloc_defdata(struct tcf_defact *d, char *defdata) { kfree(d->tcfd_defdata); - return alloc_defdata(d, datalen, defdata); + return alloc_defdata(d, defdata); } static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = { [TCA_DEF_PARMS] = { .len = sizeof(struct tc_defact) }, + [TCA_DEF_DATA] = { .type = NLA_STRING, .len = SIMP_MAX_DATA }, }; static int tcf_simp_init(struct nlattr *nla, struct nlattr *est, @@ -95,28 +97,24 @@ static int tcf_simp_init(struct nlattr *nla, struct nlattr *est, struct tc_defact *parm; struct tcf_defact *d; struct tcf_common *pc; - void *defdata; - u32 datalen = 0; + char *defdata; int ret = 0, err; if (nla == NULL) return -EINVAL; - err = nla_parse_nested(tb, TCA_DEF_MAX, nla, NULL); + err = nla_parse_nested(tb, TCA_DEF_MAX, nla, simple_policy); if (err < 0) return err; if (tb[TCA_DEF_PARMS] == NULL) return -EINVAL; - parm = nla_data(tb[TCA_DEF_PARMS]); - defdata = nla_data(tb[TCA_DEF_DATA]); - if (defdata == NULL) + if (tb[TCA_DEF_DATA] == NULL) return -EINVAL; - datalen = nla_len(tb[TCA_DEF_DATA]); - if (datalen == 0) - return -EINVAL; + parm = nla_data(tb[TCA_DEF_PARMS]); + defdata = nla_data(tb[TCA_DEF_DATA]); pc = tcf_hash_check(parm->index, a, bind, &simp_hash_info); if (!pc) { @@ -126,7 +124,7 @@ static int tcf_simp_init(struct nlattr *nla, struct nlattr *est, return -ENOMEM; d = to_defact(pc); - ret = alloc_defdata(d, datalen, defdata); + ret = alloc_defdata(d, defdata); if (ret < 0) { kfree(pc); return ret; @@ -138,7 +136,7 @@ static int tcf_simp_init(struct nlattr *nla, struct nlattr *est, tcf_simp_release(d, bind); return -EEXIST; } - realloc_defdata(d, datalen, defdata); + realloc_defdata(d, defdata); } spin_lock_bh(&d->tcf_lock); @@ -172,7 +170,7 @@ static inline int tcf_simp_dump(struct sk_buff *skb, struct tc_action *a, opt.bindcnt = d->tcf_bindcnt - bind; opt.action = d->tcf_action; NLA_PUT(skb, TCA_DEF_PARMS, sizeof(opt), &opt); - NLA_PUT(skb, TCA_DEF_DATA, d->tcfd_datalen, d->tcfd_defdata); + NLA_PUT_STRING(skb, TCA_DEF_DATA, d->tcfd_defdata); t.install = jiffies_to_clock_t(jiffies - d->tcf_tm.install); t.lastuse = jiffies_to_clock_t(jiffies - d->tcf_tm.lastuse); t.expires = jiffies_to_clock_t(d->tcf_tm.expires); -- cgit v1.2.3 From 9d1045ad68fcccfaf1393cc463ab6357693e8d1d Mon Sep 17 00:00:00 2001 From: Jamal Hadi Salim Date: Tue, 6 May 2008 00:10:24 -0700 Subject: net_cls_act: act_simple dont ignore realloc code reallocation of the policy data was being ignored. It could fail. Simplify so that there is no need for reallocating. Signed-off-by: Jamal Hadi Salim Signed-off-by: David S. Miller --- net/sched/act_simple.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'net/sched') diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c index 269ab51cd9b..1d421d059ca 100644 --- a/net/sched/act_simple.c +++ b/net/sched/act_simple.c @@ -79,10 +79,14 @@ static int alloc_defdata(struct tcf_defact *d, char *defdata) return 0; } -static int realloc_defdata(struct tcf_defact *d, char *defdata) +static void reset_policy(struct tcf_defact *d, char *defdata, + struct tc_defact *p) { - kfree(d->tcfd_defdata); - return alloc_defdata(d, defdata); + spin_lock_bh(&d->tcf_lock); + d->tcf_action = p->action; + memset(d->tcfd_defdata, 0, SIMP_MAX_DATA); + strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA); + spin_unlock_bh(&d->tcf_lock); } static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = { @@ -129,6 +133,7 @@ static int tcf_simp_init(struct nlattr *nla, struct nlattr *est, kfree(pc); return ret; } + d->tcf_action = parm->action; ret = ACT_P_CREATED; } else { d = to_defact(pc); @@ -136,13 +141,9 @@ static int tcf_simp_init(struct nlattr *nla, struct nlattr *est, tcf_simp_release(d, bind); return -EEXIST; } - realloc_defdata(d, defdata); + reset_policy(d, defdata, parm); } - spin_lock_bh(&d->tcf_lock); - d->tcf_action = parm->action; - spin_unlock_bh(&d->tcf_lock); - if (ret == ACT_P_CREATED) tcf_hash_insert(pc, &simp_hash_info); return ret; -- cgit v1.2.3