From: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Wed, 11 May 2016 23:11:06 +0000 (-0400)
Subject: lib: Fix connected lookup
X-Git-Tag: frr-2.0-rc1~923
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=38485402bc3dcc212f2c31abcffc4652c3a19a94;p=mirror%2Ffrr.git

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 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)