From cb6f7b153e6d76aea1636d7a63bce418195ecf35 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 22 Nov 2024 11:02:15 -0500 Subject: [PATCH] lib, zebra: Do not have duplicate memory type problems In zebra_mpls.c it has a usage of MTYPE_NH_LABEL which is defined in both lib/nexthop.c and zebra/zebra_mpls.c. The usage in zebra_mpls.c is a realloc. This leads to a crash: (gdb) bt 0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=126487246404032) at ./nptl/pthread_kill.c:44 1 __pthread_kill_internal (signo=6, threadid=126487246404032) at ./nptl/pthread_kill.c:78 2 __GI___pthread_kill (threadid=126487246404032, signo=signo@entry=6) at ./nptl/pthread_kill.c:89 3 0x0000730a1b442476 in __GI_raise (sig=6) at ../sysdeps/posix/raise.c:26 4 0x0000730a1b94fb18 in core_handler (signo=6, siginfo=0x7ffeed1e07b0, context=0x7ffeed1e0680) at lib/sigevent.c:268 5 6 __pthread_kill_implementation (no_tid=0, signo=6, threadid=126487246404032) at ./nptl/pthread_kill.c:44 7 __pthread_kill_internal (signo=6, threadid=126487246404032) at ./nptl/pthread_kill.c:78 8 __GI___pthread_kill (threadid=126487246404032, signo=signo@entry=6) at ./nptl/pthread_kill.c:89 9 0x0000730a1b442476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 10 0x0000730a1b4287f3 in __GI_abort () at ./stdlib/abort.c:79 11 0x0000730a1b9984f5 in _zlog_assert_failed (xref=0x730a1ba59480 <_xref.16>, extra=0x0) at lib/zlog.c:789 12 0x0000730a1b8f8908 in mt_count_free (mt=0x576e0edda520 , ptr=0x576e36617b80) at lib/memory.c:74 13 0x0000730a1b8f8a59 in qrealloc (mt=0x576e0edda520 , ptr=0x576e36617b80, size=16) at lib/memory.c:112 14 0x0000576e0ec85e2e in nhlfe_out_label_update (nhlfe=0x576e368895f0, nh_label=0x576e3660e9b0) at zebra/zebra_mpls.c:1462 15 0x0000576e0ec833ff in lsp_install (zvrf=0x576e3655fb50, label=17, rn=0x576e366197c0, re=0x576e3660a590) at zebra/zebra_mpls.c:224 16 0x0000576e0ec87c34 in zebra_mpls_lsp_install (zvrf=0x576e3655fb50, rn=0x576e366197c0, re=0x576e3660a590) at zebra/zebra_mpls.c:2215 17 0x0000576e0ecbb427 in rib_process_update_fib (zvrf=0x576e3655fb50, rn=0x576e366197c0, old=0x576e36619660, new=0x576e3660a590) at zebra/zebra_rib.c:1084 18 0x0000576e0ecbc230 in rib_process (rn=0x576e366197c0) at zebra/zebra_rib.c:1480 19 0x0000576e0ecbee04 in process_subq_route (lnode=0x576e368e0270, qindex=8 '\b') at zebra/zebra_rib.c:2661 20 0x0000576e0ecc0711 in process_subq (subq=0x576e3653fc80, qindex=META_QUEUE_BGP) at zebra/zebra_rib.c:3226 21 0x0000576e0ecc07f9 in meta_queue_process (dummy=0x576e3653fae0, data=0x576e3653fb80) at zebra/zebra_rib.c:3265 22 0x0000730a1b97d2a9 in work_queue_run (thread=0x7ffeed1e3f30) at lib/workqueue.c:282 23 0x0000730a1b96b039 in event_call (thread=0x7ffeed1e3f30) at lib/event.c:1996 24 0x0000730a1b8e4d2d in frr_run (master=0x576e36277e10) at lib/libfrr.c:1232 25 0x0000576e0ec35ca9 in main (argc=7, argv=0x7ffeed1e4208) at zebra/main.c:536 Clearly replacing a label stack is an operation that should be owned by lib/nexthop.c. So lets move this function into there and have zebra_mpls.c just call the function to replace the label stack. Signed-off-by: Donald Sharp --- lib/nexthop.c | 26 ++++++++++++++++++++++++++ lib/nexthop.h | 2 ++ zebra/zebra_mpls.c | 27 +-------------------------- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/lib/nexthop.c b/lib/nexthop.c index 98b05295b996..332581fbd8f4 100644 --- a/lib/nexthop.c +++ b/lib/nexthop.c @@ -581,6 +581,32 @@ void nexthop_del_labels(struct nexthop *nexthop) nexthop->nh_label_type = ZEBRA_LSP_NONE; } +void nexthop_change_labels(struct nexthop *nexthop, struct mpls_label_stack *new_stack) +{ + struct mpls_label_stack *nh_label_tmp; + uint32_t i; + + /* Enforce limit on label stack size */ + if (new_stack->num_labels > MPLS_MAX_LABELS) + new_stack->num_labels = MPLS_MAX_LABELS; + + /* Resize the array to accommodate the new label stack */ + if (new_stack->num_labels > nexthop->nh_label->num_labels) { + nh_label_tmp = XREALLOC(MTYPE_NH_LABEL, nexthop->nh_label, + sizeof(struct mpls_label_stack) + + new_stack->num_labels * sizeof(mpls_label_t)); + if (nh_label_tmp) { + nexthop->nh_label = nh_label_tmp; + nexthop->nh_label->num_labels = new_stack->num_labels; + } else + new_stack->num_labels = nexthop->nh_label->num_labels; + } + + /* Copy the label stack into the array */ + for (i = 0; i < new_stack->num_labels; i++) + nexthop->nh_label->label[i] = new_stack->label[i]; +} + void nexthop_add_srv6_seg6local(struct nexthop *nexthop, uint32_t action, const struct seg6local_context *ctx) { diff --git a/lib/nexthop.h b/lib/nexthop.h index 02ea4d96f2df..5dfb58d84661 100644 --- a/lib/nexthop.h +++ b/lib/nexthop.h @@ -154,6 +154,8 @@ void nexthops_free(struct nexthop *nexthop); void nexthop_add_labels(struct nexthop *nexthop, enum lsp_types_t ltype, uint8_t num_labels, const mpls_label_t *labels); void nexthop_del_labels(struct nexthop *); +void nexthop_change_labels(struct nexthop *nexthop, struct mpls_label_stack *new_stack); + void nexthop_add_srv6_seg6local(struct nexthop *nexthop, uint32_t action, const struct seg6local_context *ctx); void nexthop_del_srv6_seg6local(struct nexthop *nexthop); diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c index 9549af5f14bc..0d3fd2a7268f 100644 --- a/zebra/zebra_mpls.c +++ b/zebra/zebra_mpls.c @@ -37,7 +37,6 @@ DEFINE_MTYPE_STATIC(ZEBRA, LSP, "MPLS LSP object"); DEFINE_MTYPE_STATIC(ZEBRA, FEC, "MPLS FEC object"); DEFINE_MTYPE_STATIC(ZEBRA, NHLFE, "MPLS nexthop object"); -DEFINE_MTYPE_STATIC(ZEBRA, NH_LABEL, "Nexthop label"); bool mpls_enabled; bool mpls_pw_reach_strict; /* Strict reachability checking */ @@ -1453,31 +1452,7 @@ static int nhlfe_del(struct zebra_nhlfe *nhlfe) static void nhlfe_out_label_update(struct zebra_nhlfe *nhlfe, struct mpls_label_stack *nh_label) { - struct mpls_label_stack *nh_label_tmp; - int i; - - /* Enforce limit on label stack size */ - if (nh_label->num_labels > MPLS_MAX_LABELS) - nh_label->num_labels = MPLS_MAX_LABELS; - - /* Resize the array to accommodate the new label stack */ - if (nh_label->num_labels > nhlfe->nexthop->nh_label->num_labels) { - nh_label_tmp = XREALLOC(MTYPE_NH_LABEL, nhlfe->nexthop->nh_label, - sizeof(struct mpls_label_stack) + - nh_label->num_labels * - sizeof(mpls_label_t)); - if (nh_label_tmp) { - nhlfe->nexthop->nh_label = nh_label_tmp; - nhlfe->nexthop->nh_label->num_labels = - nh_label->num_labels; - } else - nh_label->num_labels = - nhlfe->nexthop->nh_label->num_labels; - } - - /* Copy the label stack into the array */ - for (i = 0; i < nh_label->num_labels; i++) - nhlfe->nexthop->nh_label->label[i] = nh_label->label[i]; + nexthop_change_labels(nhlfe->nexthop, nh_label); } static int mpls_lsp_uninstall_all(struct hash *lsp_table, struct zebra_lsp *lsp,