diff options
author | Claudio Jeker <claudio@cvs.openbsd.org> | 2010-04-20 09:02:13 +0000 |
---|---|---|
committer | Claudio Jeker <claudio@cvs.openbsd.org> | 2010-04-20 09:02:13 +0000 |
commit | fbf276b9c9db3385d53b0a4985e7369632824758 (patch) | |
tree | 364b43e2de3ce478896e0eb7a29cc3e03ada1925 | |
parent | 8ca3690f374266d9f1243632b87f906b80bb4fff (diff) |
prefix_unlink() must remove the rib entry. Currently this was only done
in prefix_destroy() but there is another caller of prefix_unlink() which
missed the rib_remove() resulting in tree corruption and possible crashes.
Doing the remove in prefix_unlink() is better since we do the same with the
prefix and rib & prefix are linked. Fix some comments to match code and
remove double call to pt_empty()/pt_remove().
Found while hacking on something else.
-rw-r--r-- | usr.sbin/bgpd/rde_rib.c | 27 |
1 files changed, 10 insertions, 17 deletions
diff --git a/usr.sbin/bgpd/rde_rib.c b/usr.sbin/bgpd/rde_rib.c index 9a54f8aa1f5..abada750b58 100644 --- a/usr.sbin/bgpd/rde_rib.c +++ b/usr.sbin/bgpd/rde_rib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde_rib.c,v 1.125 2010/04/07 09:44:11 claudio Exp $ */ +/* $OpenBSD: rde_rib.c,v 1.126 2010/04/20 09:02:12 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> @@ -921,16 +921,12 @@ prefix_updateall(struct rde_aspath *asp, enum nexthop_state state, void prefix_destroy(struct prefix *p) { - struct rib_entry *re; struct rde_aspath *asp; - re = p->rib; asp = p->aspath; prefix_unlink(p); prefix_free(p); - if (rib_empty(re)) - rib_remove(re); if (path_empty(asp)) path_destroy(asp); } @@ -943,7 +939,6 @@ prefix_network_clean(struct rde_peer *peer, time_t reloadtime, u_int32_t flags) { struct rde_aspath *asp, *xasp; struct prefix *p, *xp; - struct pt_entry *pte; for (asp = LIST_FIRST(&peer->path_h); asp != NULL; asp = xasp) { xasp = LIST_NEXT(asp, peer_l); @@ -952,12 +947,8 @@ prefix_network_clean(struct rde_peer *peer, time_t reloadtime, u_int32_t flags) for (p = LIST_FIRST(&asp->prefix_h); p != NULL; p = xp) { xp = LIST_NEXT(p, path_l); if (reloadtime > p->lastchange) { - pte = p->prefix; prefix_unlink(p); prefix_free(p); - - if (pt_empty(pte)) - pt_remove(pte); } } if (path_empty(asp)) @@ -990,11 +981,11 @@ prefix_link(struct prefix *pref, struct rib_entry *re, struct rde_aspath *asp) static void prefix_unlink(struct prefix *pref) { - if (pref->rib) { - /* make route decision */ - LIST_REMOVE(pref, rib_l); - prefix_evaluate(NULL, pref->rib); - } + struct rib_entry *re = pref->rib; + + /* make route decision */ + LIST_REMOVE(pref, rib_l); + prefix_evaluate(NULL, re); LIST_REMOVE(pref, path_l); PREFIX_COUNT(pref->aspath, -1); @@ -1002,6 +993,8 @@ prefix_unlink(struct prefix *pref) pt_unref(pref->prefix); if (pt_empty(pref->prefix)) pt_remove(pref->prefix); + if (rib_empty(re)) + rib_remove(re); /* destroy all references to other objects */ pref->aspath = NULL; @@ -1009,8 +1002,8 @@ prefix_unlink(struct prefix *pref) pref->rib = NULL; /* - * It's the caller's duty to remove empty aspath respectively pt_entry - * structures. Also freeing the unlinked prefix is the caller's duty. + * It's the caller's duty to remove empty aspath structures. + * Also freeing the unlinked prefix is the caller's duty. */ } |