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 <sharpd@cumulusnetworks.com>
Reviewed-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>

diff --git a/lib/if.c b/lib/if.c
index ebd8f5e02db656d89f67e71d7d45dd4c681d254c..d49b5acad73a0e8d49095233616a4587fb42fd6b 100644 (file)
--- 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 78a73fadaf15a4671cef61f9061037b0f728ceee..4ec85bc84179ebe5b5db3b34b642aee40682bfb8 100644 (file)
--- 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 4351a67bee50424da95ef5752ed0dfdf24140b35..d1e9a96546f15a87110732111b3c3601416c8649 100644 (file)
--- 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)