From ecea0cb083a757664a3c105194fba06405f96e58 Mon Sep 17 00:00:00 2001 From: Jafar Al-Gharaibeh Date: Thu, 21 Apr 2016 16:22:33 -0500 Subject: [PATCH] ospfd: fix - correct neighbor index on changing/p2p/virtual links ospfd keeps a list of neighbor routers for each configured interface. This list is indexed using the neighbor router id in case of point-to-point and virtual link types, otherwise the list is indexed using the neighbor's source IP (RFC 2328, page 96). The router adds itself as a "pseudo" neighbor on each link, and also keeps a pointer called (nbr_self) to the neighbor structure. This takes place when the interface is first configured. Currently ospfd adds this pseudo neighbor before the link parameters are fully configure, including whether the link type is point-to-point or virtual link. This causes the pseudo neighbor to be always indexed using the source IP address regardless of th link type. For point-to-point and virtual links, this causes the lookup for the pseudo neighbor to always fail because the lookup is done using the router id whereas the neighbor was added using its source IP address. This becomes really problematic if there is a state change that requires a rebuild of nbr_self, changing the router id for example. When resetting nbr_self, the router first tries to remove the pseudo neighbor form its neighbor list on each link by looking it up and resetting any references to it before freeing the neighbor structure. since the lookup fails to retrieve any references in the case of point-to-point and virtual links the neighbor structure is freed leaving dangling references to it. Any access to the neighbor list after that is bound to stumble over this dangling pointer causing ospfd to crash. Signed-off-by: Jafar Al-Gharaibeh Tested-by: NetDEF CI System (cherry picked from commit bb01bdd740339b0c07d8ed0786811801b2a79192) --- ospfd/ospf_interface.c | 8 +++++--- ospfd/ospf_neighbor.c | 33 ++++++++++++++++++++++++++++++++- ospfd/ospfd.c | 6 +++--- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index 5b6be1c8dc..a062004ac4 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -234,8 +234,8 @@ ospf_if_new (struct ospf *ospf, struct interface *ifp, struct prefix *p) /* Set default values. */ ospf_if_reset_variables (oi); - /* Add pseudo neighbor. */ - oi->nbr_self = ospf_nbr_new (oi); + /* Set pseudo neighbor to Null */ + oi->nbr_self = NULL; oi->ls_upd_queue = route_table_init (); oi->t_ls_upd_event = NULL; @@ -925,7 +925,9 @@ ospf_vl_new (struct ospf *ospf, struct ospf_vl_data *vl_data) if (IS_DEBUG_OSPF_EVENT) zlog_debug ("ospf_vl_new(): set associated area to the backbone"); - ospf_nbr_add_self (voi); + /* Add pseudo neighbor. */ + ospf_nbr_self_reset (voi); + ospf_area_add_if (voi->area, voi); ospf_if_stream_set (voi); diff --git a/ospfd/ospf_neighbor.c b/ospfd/ospf_neighbor.c index 36251655a7..46fcc6ba2f 100644 --- a/ospfd/ospf_neighbor.c +++ b/ospfd/ospf_neighbor.c @@ -185,6 +185,35 @@ ospf_nbr_delete (struct ospf_neighbor *nbr) route_unlock_node (rn); } + else + { + /* + * This neighbor was not found, but before we move on and + * free the neighbor structre, make sure that it was not + * indexed incorrectly and ended up in the "worng" place + */ + + /* Reverse the lookup rules */ + if (oi->type == OSPF_IFTYPE_VIRTUALLINK || + oi->type == OSPF_IFTYPE_POINTOPOINT) + p.u.prefix4 = nbr->src; + else + p.u.prefix4 = nbr->router_id; + + rn = route_node_lookup (oi->nbrs, &p); + if (rn){ + /* We found the neighbor! + * Now make sure it is not the exact same neighbor + * structure that we are about to free + */ + if (nbr == rn->info){ + /* Same neighbor, drop the reference to it */ + rn->info = NULL; + route_unlock_node (rn); + } + route_unlock_node (rn); + } + } /* Free ospf_neighbor structure. */ ospf_nbr_free (nbr); @@ -211,7 +240,9 @@ ospf_nbr_bidirectional (struct in_addr *router_id, void ospf_nbr_self_reset (struct ospf_interface *oi) { - ospf_nbr_delete (oi->nbr_self); + if (oi->nbr_self) + ospf_nbr_delete (oi->nbr_self); + oi->nbr_self = ospf_nbr_new (oi); ospf_nbr_add_self (oi); } diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c index 5b5014eec8..ea66042f2e 100644 --- a/ospfd/ospfd.c +++ b/ospfd/ospfd.c @@ -879,9 +879,6 @@ add_ospf_interface (struct interface *ifp, struct ospf_area *area, oi->params = ospf_lookup_if_params (ifp, oi->address->u.prefix4); oi->output_cost = ospf_if_get_output_cost (oi); - /* Add pseudo neighbor. */ - ospf_nbr_add_self (oi); - /* Relate ospf interface to ospf instance. */ oi->ospf = area->ospf; @@ -890,6 +887,9 @@ add_ospf_interface (struct interface *ifp, struct ospf_area *area, skip network type setting. */ oi->type = IF_DEF_PARAMS (ifp)->type; + /* Add pseudo neighbor. */ + ospf_nbr_self_reset (oi); + ospf_area_add_if (oi->area, oi); return (oi); -- 2.39.5