zebra: mpls_ftn_uninstall handle nhg hash label change

We were crashing due to a missed label change code path
in mpls_ftn_uninstall() with the zebra_nhg hashing code.

Add a static handler function for label changing everywhere
in that code and use it in mpls_ftn_uninstall().

The crash was found in the ISIS-SR tests:

==23== Thread 1:
==23== Invalid read of size 4
==23==    at 0x15B20E: zebra_nhg_hash_equal (zebra_nhg.c:365)
==23==    by 0x489A2FD: hash_get (hash.c:143)
==23==    by 0x489A4BC: hash_lookup (hash.c:183)
==23==    by 0x15B5A3: zebra_nhg_find (zebra_nhg.c:494)
==23==    by 0x15C536: zebra_nhg_rib_find (zebra_nhg.c:1070)
==23==    by 0x1573E8: mpls_ftn_update (zebra_mpls.c:2661)
==23==    by 0x1A2554: zread_mpls_labels_replace (zapi_msg.c:1890)
==23==    by 0x1A41CD: zserv_handle_commands (zapi_msg.c:2613)
==23==    by 0x199B17: zserv_process_messages (zserv.c:517)
==23==    by 0x48EE6B7: thread_call (thread.c:1549)
==23==    by 0x48A8AD5: frr_run (libfrr.c:1064)
==23==    by 0x1391B7: main (main.c:468)
==23==  Address 0x5839330 is 0 bytes inside a block of size 80 free'd
==23==    at 0x48369AB: free (vg_replace_malloc.c:530)
==23==    by 0x48AEE6C: qfree (memory.c:129)
==23==    by 0x15C5F8: zebra_nhg_free (zebra_nhg.c:1095)
==23==    by 0x15BC8C: zebra_nhg_handle_uninstall (zebra_nhg.c:734)
==23==    by 0x15DCFA: zebra_nhg_uninstall_kernel (zebra_nhg.c:1826)
==23==    by 0x15C666: zebra_nhg_decrement_ref (zebra_nhg.c:1106)
==23==    by 0x15D9D7: zebra_nhg_re_update_ref (zebra_nhg.c:1711)
==23==    by 0x15D8B1: nexthop_active_update (zebra_nhg.c:1660)
==23==    by 0x167072: rib_process (zebra_rib.c:1154)
==23==    by 0x168D72: process_subq_route (zebra_rib.c:2039)
==23==    by 0x168E92: process_subq (zebra_rib.c:2078)
==23==    by 0x168F5B: meta_queue_process (zebra_rib.c:2112)
==23==  Block was alloc'd at
==23==    at 0x4837B65: calloc (vg_replace_malloc.c:752)
==23==    by 0x48AED56: qcalloc (memory.c:110)
==23==    by 0x15B07B: zebra_nhg_copy (zebra_nhg.c:307)
==23==    by 0x15B13E: zebra_nhg_hash_alloc (zebra_nhg.c:329)
==23==    by 0x489A339: hash_get (hash.c:148)
==23==    by 0x15B6CA: zebra_nhg_find (zebra_nhg.c:532)
==23==    by 0x15C536: zebra_nhg_rib_find (zebra_nhg.c:1070)
==23==    by 0x15D89A: nexthop_active_update (zebra_nhg.c:1658)
==23==    by 0x167072: rib_process (zebra_rib.c:1154)
==23==    by 0x168D72: process_subq_route (zebra_rib.c:2039)
==23==    by 0x168E92: process_subq (zebra_rib.c:2078)
==23==    by 0x168F5B: meta_queue_process (zebra_rib.c:2112)

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>

diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c
index ef1bd02608be3c13344ab4d03f164d62f5745873..6942a379894b1180173cf1e6086a896ad4065f91 100644 (file)
--- a/zebra/zebra_mpls.c
+++ b/zebra/zebra_mpls.c
@@ -2565,6 +2565,16 @@ void zebra_mpls_print_fec(struct vty *vty, struct zebra_vrf *zvrf,
        fec_print(rn->info, vty);
 }
 
+static void mpls_zebra_nhg_update(struct route_entry *re, afi_t afi,
+                                 struct nexthop_group *new_grp)
+{
+       struct nhg_hash_entry *nhe;
+
+       nhe = zebra_nhg_rib_find(0, new_grp, afi);
+
+       zebra_nhg_re_update_ref(re, nhe);
+}
+
 static bool mpls_ftn_update_nexthop(int add, struct nexthop *nexthop,
                                    enum lsp_types_t type, mpls_label_t label)
 {
@@ -2591,7 +2601,6 @@ int mpls_ftn_update(int add, struct zebra_vrf *zvrf, enum lsp_types_t type,
        struct route_entry *re;
        struct nexthop *nexthop;
        struct nexthop_group new_grp = {};
-       struct nhg_hash_entry *nhe = NULL;
        bool found;
        afi_t afi = family2afi(prefix->family);
 
@@ -2658,12 +2667,11 @@ int mpls_ftn_update(int add, struct zebra_vrf *zvrf, enum lsp_types_t type,
        }
 
        if (found) {
-               nhe = zebra_nhg_rib_find(0, &new_grp, afi);
-
-               zebra_nhg_re_update_ref(re, nhe);
-
                SET_FLAG(re->status, ROUTE_ENTRY_CHANGED);
                SET_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED);
+
+               mpls_zebra_nhg_update(re, afi, &new_grp);
+
                rib_queue_add(rn);
        }
 
@@ -2680,10 +2688,11 @@ int mpls_ftn_uninstall(struct zebra_vrf *zvrf, enum lsp_types_t type,
        struct route_node *rn;
        struct route_entry *re;
        struct nexthop *nexthop;
+       struct nexthop_group new_grp = {};
+       afi_t afi = family2afi(prefix->family);
 
        /* Lookup table.  */
-       table = zebra_vrf_table(family2afi(prefix->family), SAFI_UNICAST,
-                               zvrf_id(zvrf));
+       table = zebra_vrf_table(afi, SAFI_UNICAST, zvrf_id(zvrf));
        if (!table)
                return -1;
 
@@ -2698,11 +2707,18 @@ int mpls_ftn_uninstall(struct zebra_vrf *zvrf, enum lsp_types_t type,
        if (re == NULL)
                return -1;
 
-       for (nexthop = re->ng->nexthop; nexthop; nexthop = nexthop->next)
+       nexthop_group_copy(&new_grp, re->ng);
+
+       for (nexthop = new_grp.nexthop; nexthop; nexthop = nexthop->next)
                nexthop_del_labels(nexthop);
 
        SET_FLAG(re->status, ROUTE_ENTRY_CHANGED);
        SET_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED);
+
+       mpls_zebra_nhg_update(re, afi, &new_grp);
+
+       nexthops_free(new_grp.nexthop);
+
        rib_queue_add(rn);
 
        return 0;
@@ -2904,7 +2920,6 @@ static void mpls_ftn_uninstall_all(struct zebra_vrf *zvrf,
                update = 0;
                RNODE_FOREACH_RE (rn, re) {
                        struct nexthop_group new_grp = {};
-                       struct nhg_hash_entry *nhe = NULL;
 
                        nexthop_group_copy(&new_grp, re->ng);
 
@@ -2920,11 +2935,8 @@ static void mpls_ftn_uninstall_all(struct zebra_vrf *zvrf,
                                update = 1;
                        }
 
-                       if (CHECK_FLAG(re->status,
-                                      ROUTE_ENTRY_LABELS_CHANGED)) {
-                               nhe = zebra_nhg_rib_find(0, &new_grp, afi);
-                               zebra_nhg_re_update_ref(re, nhe);
-                       }
+                       if (CHECK_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED))
+                               mpls_zebra_nhg_update(re, afi, &new_grp);
 
                        nexthops_free(new_grp.nexthop);
                }