Messages in this thread Patch in this message | | | Subject | Re: [PATCH] fib:fix BUG_ON in fib_nl_newrule when add new fib rule | From | Eric Dumazet <> | Date | Sun, 11 Sep 2011 09:12:35 +0200 |
| |
Le dimanche 11 septembre 2011 à 01:19 +0800, Wanlong Gao a écrit : > From: Gao feng <gaofeng@cn.fujitsu.com> > > add new fib rule can cause BUG_ON > the reproduce shell is > > #ip rule add pref 38 > #ip rule add pref 38 > #ip rule add to 192.168.3.0/24 goto 38 > #ip rule del pref 38 > #ip rule add to 192.168.3.0/24 goto 38 > #ip rule add pref 38 > > then the BUG_ON will happen > add a var unresolved in struct fib_rule > and use it to identify whether this rule is unresolved > > Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> > --- > include/net/fib_rules.h | 1 + > net/core/fib_rules.c | 5 ++++- > 2 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h > index 075f1e3..e4bae01 100644 > --- a/include/net/fib_rules.h > +++ b/include/net/fib_rules.h > @@ -19,6 +19,7 @@ struct fib_rule { > u32 flags; > u32 table; > u8 action; > + u8 unresolved; > u32 target; > struct fib_rule __rcu *ctarget; > char iifname[IFNAMSIZ]; > diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c > index e7ab0c0..aa20560 100644 > --- a/net/core/fib_rules.c > +++ b/net/core/fib_rules.c > @@ -384,9 +384,11 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg) > */ > list_for_each_entry(r, &ops->rules_list, list) { > if (r->action == FR_ACT_GOTO && > - r->target == rule->pref) { > + r->target == rule->pref && > + r->unresolved) { > BUG_ON(rtnl_dereference(r->ctarget) != NULL); > rcu_assign_pointer(r->ctarget, rule); > + r->unresolved = 0; > if (--ops->unresolved_rules == 0) > break; > } > @@ -488,6 +490,7 @@ static int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg) > list_for_each_entry(tmp, &ops->rules_list, list) { > if (rtnl_dereference(tmp->ctarget) == rule) { > rcu_assign_pointer(tmp->ctarget, NULL); > + tmp->unresolved = 1; > ops->unresolved_rules++; > } > }
Hmm, good catch, but you add 'unresolved' field but dont init it correctly to 1 in all cases.
Following will break :
ip rule add pref 100 ip rule add to 192.168.3.0/24 goto 200 ip rule add pref 200
Normally, we should have
unresolved = (ctarget == NULL);
What about following patch instead ?
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index e7ab0c0..3231b46 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -384,8 +384,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg) */ list_for_each_entry(r, &ops->rules_list, list) { if (r->action == FR_ACT_GOTO && - r->target == rule->pref) { - BUG_ON(rtnl_dereference(r->ctarget) != NULL); + r->target == rule->pref && + rtnl_dereference(r->ctarget) == NULL) { rcu_assign_pointer(r->ctarget, rule); if (--ops->unresolved_rules == 0) break;
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |