From 38485402bc3dcc212f2c31abcffc4652c3a19a94 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 11 May 2016 19:11:06 -0400 Subject: [PATCH] lib: Fix connected lookup When looking up the connected route, the delete was causing crashes in OSPF due to the oi having copies of the freshly deleted connected interface. Fix code to first lookup the connected route and use that instead of just deleting it. Valgrind Findings: ==24112== Invalid read of size 1 ==24112== at 0x4E8283F: ospf_intra_add_stub (ospf_route.c:614) ==24112== by 0x4E80B15: ospf_spf_process_stubs (ospf_spf.c:1064) ==24112== by 0x4E80F74: ospf_spf_calculate (ospf_spf.c:1269) ==24112== by 0x4E811C9: ospf_spf_calculate_timer (ospf_spf.c:1339) ==24112== by 0x5126230: thread_call (thread.c:1577) ==24112== by 0x401E00: main (ospf_main.c:377) ==24112== Address 0x7f56a09 is 9 bytes inside a block of size 40 free'd ==24112== at 0x4C29E90: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==24112== by 0x51290B3: zfree (memory.c:132) ==24112== by 0x51287F0: connected_free (if.c:987) ==24112== by 0x514406A: zebra_interface_address_read (zclient.c:1146) ==24112== by 0x4E5A81C: ospf_interface_address_add (ospf_zebra.c:262) ==24112== by 0x5144838: zclient_read (zclient.c:1397) ==24112== by 0x5126230: thread_call (thread.c:1577) ==24112== by 0x401E00: main (ospf_main.c:377) Ticket: CM-10890 Signed-off-by: Donald Sharp Reviewed-by: Vivek Venkatraman Reviewed-by: Daniel Walton --- lib/if.c | 18 ++++++++++++++++++ lib/if.h | 2 ++ lib/zclient.c | 19 ++++++++----------- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/lib/if.c b/lib/if.c index ebd8f5e02d..d49b5acad7 100644 --- a/lib/if.c +++ b/lib/if.c @@ -1081,6 +1081,24 @@ connected_same_prefix (struct prefix *p1, struct prefix *p2) return 0; } +struct connected * +connected_lookup_prefix_exact (struct interface *ifp, struct prefix *p) +{ + struct listnode *node; + struct listnode *next; + struct connected *ifc; + + for (node = listhead (ifp->connected); node; node = next) + { + ifc = listgetdata (node); + next = node->next; + + if (connected_same_prefix (ifc->address, p)) + return ifc; + } + return NULL; +} + struct connected * connected_delete_by_prefix (struct interface *ifp, struct prefix *p) { diff --git a/lib/if.h b/lib/if.h index 78a73fadaf..4ec85bc841 100644 --- a/lib/if.h +++ b/lib/if.h @@ -352,6 +352,8 @@ extern struct connected *connected_delete_by_prefix (struct interface *, struct prefix *); extern struct connected *connected_lookup_prefix (struct interface *, struct prefix *); +extern struct connected *connected_lookup_prefix_exact (struct interface *, + struct prefix *); extern struct nbr_connected *nbr_connected_new (void); extern void nbr_connected_free (struct nbr_connected *); struct nbr_connected *nbr_connected_check (struct interface *, struct prefix *); diff --git a/lib/zclient.c b/lib/zclient.c index 4351a67bee..d1e9a96546 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -1138,17 +1138,14 @@ zebra_interface_address_read (int type, struct stream *s, vrf_id_t vrf_id) if (type == ZEBRA_INTERFACE_ADDRESS_ADD) { - /* We have situations where the address may be replayed more than once. - * Check and delete older matching address, first. - */ - ifc = connected_delete_by_prefix(ifp, &p); - if (ifc) - connected_free (ifc); - - /* N.B. NULL destination pointers are encoded as all zeroes */ - ifc = connected_add_by_prefix(ifp, &p,(memconstant(&d.u.prefix,0,plen) ? - NULL : &d)); - if (ifc != NULL) + ifc = connected_lookup_prefix_exact (ifp, &p); + if (!ifc) + { + /* N.B. NULL destination pointers are encoded as all zeroes */ + ifc = connected_add_by_prefix(ifp, &p, (memconstant(&d.u.prefix,0,plen) ? + NULL : &d)); + } + if (ifc) { ifc->flags = ifc_flags; if (ifc->destination) -- 2.39.5