isisd, lib, ospfd, pathd: Null out free'd pointer

The commands:

router isis 1
  mpls-te on
  no mpls-te on
  mpls-te on
  no mpls-te on
!

Will crash

Valgrind gives us this:
==652336== Invalid read of size 8
==652336==    at 0x49AB25C: typed_rb_min (typerb.c:495)
==652336==    by 0x4943B54: vertices_const_first (link_state.h:424)
==652336==    by 0x493DCE4: vertices_first (link_state.h:424)
==652336==    by 0x493DADC: ls_ted_del_all (link_state.c:1010)
==652336==    by 0x47E77B: isis_instance_mpls_te_destroy (isis_nb_config.c:1871)
==652336==    by 0x495BE20: nb_callback_destroy (northbound.c:1131)
==652336==    by 0x495B5AC: nb_callback_configuration (northbound.c:1356)
==652336==    by 0x4958127: nb_transaction_process (northbound.c:1473)
==652336==    by 0x4958275: nb_candidate_commit_apply (northbound.c:906)
==652336==    by 0x49585B8: nb_candidate_commit (northbound.c:938)
==652336==    by 0x495CE4A: nb_cli_classic_commit (northbound_cli.c:64)
==652336==    by 0x495D6C5: nb_cli_apply_changes_internal (northbound_cli.c:250)
==652336==  Address 0x6f928e0 is 272 bytes inside a block of size 320 free'd
==652336==    at 0x48399AB: free (vg_replace_malloc.c:538)
==652336==    by 0x494BA30: qfree (memory.c:141)
==652336==    by 0x493D99D: ls_ted_del (link_state.c:997)
==652336==    by 0x493DC20: ls_ted_del_all (link_state.c:1018)
==652336==    by 0x47E77B: isis_instance_mpls_te_destroy (isis_nb_config.c:1871)
==652336==    by 0x495BE20: nb_callback_destroy (northbound.c:1131)
==652336==    by 0x495B5AC: nb_callback_configuration (northbound.c:1356)
==652336==    by 0x4958127: nb_transaction_process (northbound.c:1473)
==652336==    by 0x4958275: nb_candidate_commit_apply (northbound.c:906)
==652336==    by 0x49585B8: nb_candidate_commit (northbound.c:938)
==652336==    by 0x495CE4A: nb_cli_classic_commit (northbound_cli.c:64)
==652336==    by 0x495D6C5: nb_cli_apply_changes_internal (northbound_cli.c:250)
==652336==  Block was alloc'd at
==652336==    at 0x483AB65: calloc (vg_replace_malloc.c:760)
==652336==    by 0x494B6F8: qcalloc (memory.c:116)
==652336==    by 0x493D7D2: ls_ted_new (link_state.c:967)
==652336==    by 0x47E4DD: isis_instance_mpls_te_create (isis_nb_config.c:1832)
==652336==    by 0x495BB29: nb_callback_create (northbound.c:1034)
==652336==    by 0x495B547: nb_callback_configuration (northbound.c:1348)
==652336==    by 0x4958127: nb_transaction_process (northbound.c:1473)
==652336==    by 0x4958275: nb_candidate_commit_apply (northbound.c:906)
==652336==    by 0x49585B8: nb_candidate_commit (northbound.c:938)
==652336==    by 0x495CE4A: nb_cli_classic_commit (northbound_cli.c:64)
==652336==    by 0x495D6C5: nb_cli_apply_changes_internal (northbound_cli.c:250)
==652336==    by 0x495D23E: nb_cli_apply_changes (northbound_cli.c:268)

Let's null out the pointer.  After this change.  Valgrind no longer reports issues
and isisd no longer crashes.

Fixes: #10939
Signed-off-by: Donald Sharp <sharpd@nvidia.com>

diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c
index 019c26687bd86330502bfddc05d7a6565edac835..de7797813a4262d064edacfff80881502d2d74a1 100644 (file)
--- a/isisd/isis_nb_config.c
+++ b/isisd/isis_nb_config.c
@@ -1868,7 +1868,7 @@ int isis_instance_mpls_te_destroy(struct nb_cb_destroy_args *args)
                return NB_OK;
 
        /* Remove Link State Database */
-       ls_ted_del_all(area->mta->ted);
+       ls_ted_del_all(&area->mta->ted);
 
        /* Flush LSP if circuit engage */
        for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit)) {

diff --git a/lib/link_state.c b/lib/link_state.c
index e4ccd0fb6577486533698a0fc5c233a58d1cb6a5..639a1d37d8a152cc69d43db4786434fed8566dfe 100644 (file)
--- a/lib/link_state.c
+++ b/lib/link_state.c
@@ -997,25 +997,26 @@ void ls_ted_del(struct ls_ted *ted)
        XFREE(MTYPE_LS_DB, ted);
 }
 
-void ls_ted_del_all(struct ls_ted *ted)
+void ls_ted_del_all(struct ls_ted **ted)
 {
        struct ls_vertex *vertex;
        struct ls_edge *edge;
        struct ls_subnet *subnet;
 
-       if (ted == NULL)
+       if (*ted == NULL)
                return;
 
        /* First remove Vertices, Edges and Subnets and associated Link State */
-       frr_each_safe (vertices, &ted->vertices, vertex)
-               ls_vertex_del_all(ted, vertex);
-       frr_each_safe (edges, &ted->edges, edge)
-               ls_edge_del_all(ted, edge);
-       frr_each_safe (subnets, &ted->subnets, subnet)
-               ls_subnet_del_all(ted, subnet);
+       frr_each_safe (vertices, &(*ted)->vertices, vertex)
+               ls_vertex_del_all(*ted, vertex);
+       frr_each_safe (edges, &(*ted)->edges, edge)
+               ls_edge_del_all(*ted, edge);
+       frr_each_safe (subnets, &(*ted)->subnets, subnet)
+               ls_subnet_del_all(*ted, subnet);
 
        /* then remove TED itself */
-       ls_ted_del(ted);
+       ls_ted_del(*ted);
+       *ted = NULL;
 }
 
 void ls_ted_clean(struct ls_ted *ted)

diff --git a/lib/link_state.h b/lib/link_state.h
index 761e8b6a2771dacb993de39bcec4d3f9510c6b4c..f46a2068a1d13eec4b196f5905c8bf2294756c41 100644 (file)
--- a/lib/link_state.h
+++ b/lib/link_state.h
@@ -746,7 +746,7 @@ extern void ls_ted_del(struct ls_ted *ted);
  *
  * @param ted  Link State Data Base
  */
-extern void ls_ted_del_all(struct ls_ted *ted);
+extern void ls_ted_del_all(struct ls_ted **ted);
 
 /**
  * Clean Link State Data Base by removing all Vertices, Edges and SubNets marked

diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c
index 999bc49d918f56983fe9c4de98d61e99f28745fa..2679873674871cb81ce058645d69ea334e6f65a8 100644 (file)
--- a/ospfd/ospf_te.c
+++ b/ospfd/ospf_te.c
@@ -3908,7 +3908,7 @@ DEFUN (no_ospf_mpls_te,
        ote_debug("MPLS-TE: ON -> OFF");
 
        /* Remove TED */
-       ls_ted_del_all(OspfMplsTE.ted);
+       ls_ted_del_all(&OspfMplsTE.ted);
        OspfMplsTE.enabled = false;
 
        /* Flush all TE Opaque LSAs */

diff --git a/pathd/path_ted.c b/pathd/path_ted.c
index 3440b933991c86cbed2f8245550eab99be69e9dc..b30f38f6fb3d857e13b55978ed17d16d0d5205e4 100644 (file)
--- a/pathd/path_ted.c
+++ b/pathd/path_ted.c
@@ -66,7 +66,7 @@ uint32_t path_ted_teardown(void)
        PATH_TED_DEBUG("%s : TED [%p]", __func__, ted_state_g.ted);
        path_ted_unregister_vty();
        path_ted_stop_importing_igp();
-       ls_ted_del_all(ted_state_g.ted);
+       ls_ted_del_all(&ted_state_g.ted);
        path_ted_timer_sync_cancel();
        path_ted_timer_refresh_cancel();
        return 0;
@@ -391,7 +391,7 @@ DEFUN (no_path_ted,
        }
 
        /* Remove TED */
-       ls_ted_del_all(ted_state_g.ted);
+       ls_ted_del_all(&ted_state_g.ted);
        ted_state_g.enabled = false;
        PATH_TED_DEBUG("%s: PATHD-TED: ON -> OFF", __func__);
        ted_state_g.import = IMPORT_UNKNOWN;