Skip to content

Commit

Permalink
ospfd: Fix opaque functab leak and opaque AS cleanup problems
Browse files Browse the repository at this point in the history 
   1. Fix ospf opaque LSA function table memory leak.
   2. Remove incorrect one-to-one association of OSPF info-per-type
      to function table (since there many be many).
   3. Fix a problem with opaque AS external cleanup that was exposed
      by #2.
   4. Fix LSA memory leak in ospf_opaque_type9_lsa_if_cleanup().

Signed-off-by: Acee <[email protected]>
  • Loading branch information
Acee committed Dec 20, 2023
1 parent b6cb72f commit dec87fa
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 31 deletions.
2 changes: 1 addition & 1 deletion ospfd/ospf_apiserver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2092,7 +2092,7 @@ void ospf_apiserver_flush_opaque_lsa(struct ospf_apiserver *apiserv,
lsa, (void *)&param, 0);
break;
case OSPF_OPAQUE_AS_LSA:
LSDB_LOOP (OPAQUE_LINK_LSDB(ospf), rn, lsa)
LSDB_LOOP (OPAQUE_AS_LSDB(ospf), rn, lsa)
apiserver_flush_opaque_type_callback(lsa,
(void *)&param, 0);
break;
Expand Down
101 changes: 71 additions & 30 deletions ospfd/ospf_opaque.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ static void free_opaque_info_per_type(struct opaque_info_per_type *oipt,

struct ospf_opaque_functab {
uint8_t opaque_type;
struct opaque_info_per_type *oipt;
uint32_t ref_count;

int (*new_if_hook)(struct interface *ifp);
int (*del_if_hook)(struct interface *ifp);
Expand All @@ -280,16 +280,38 @@ static struct list *ospf_opaque_type9_funclist;
static struct list *ospf_opaque_type10_funclist;
static struct list *ospf_opaque_type11_funclist;

static void ospf_opaque_functab_ref(struct ospf_opaque_functab *functab)
{
functab->ref_count++;
}

static void ospf_opaque_functab_deref(struct ospf_opaque_functab *functab)
{
assert(functab->ref_count);
functab->ref_count--;
if (functab->ref_count == 0)
XFREE(MTYPE_OSPF_OPAQUE_FUNCTAB, functab);
}

static void ospf_opaque_del_functab(void *val)
{
XFREE(MTYPE_OSPF_OPAQUE_FUNCTAB, val);
struct ospf_opaque_functab *functab = (struct ospf_opaque_functab *)val;

if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Opaque LSA functab list deletion callback type %u (%p)",
__func__, functab->opaque_type, functab);

ospf_opaque_functab_deref(functab);
return;
}

static void ospf_opaque_funclist_init(void)
{
struct list *funclist;

if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Function list initialize", __func__);

funclist = ospf_opaque_wildcard_funclist = list_new();
funclist->del = ospf_opaque_del_functab;

Expand All @@ -308,6 +330,9 @@ static void ospf_opaque_funclist_term(void)
{
struct list *funclist;

if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Function list terminate", __func__);

funclist = ospf_opaque_wildcard_funclist;
list_delete(&funclist);

Expand Down Expand Up @@ -383,18 +408,24 @@ int ospf_register_opaque_functab(

for (ALL_LIST_ELEMENTS(funclist, node, nnode, functab))
if (functab->opaque_type == opaque_type) {
flog_warn(
EC_OSPF_LSA,
"%s: Duplicated entry?: lsa_type(%u), opaque_type(%u)",
__func__, lsa_type, opaque_type);
return -1;
if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Opaque LSA functab found type %u, (%p)",
__func__, functab->opaque_type,
functab);
break;
}

new = XCALLOC(MTYPE_OSPF_OPAQUE_FUNCTAB,
sizeof(struct ospf_opaque_functab));
if (functab == NULL)
new = XCALLOC(MTYPE_OSPF_OPAQUE_FUNCTAB,
sizeof(struct ospf_opaque_functab));
else {
if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Re-register Opaque LSA type %u, opaque type %u, (%p)",
__func__, lsa_type, opaque_type, functab);
return 0;
}

new->opaque_type = opaque_type;
new->oipt = NULL;
new->new_if_hook = new_if_hook;
new->del_if_hook = del_if_hook;
new->ism_change_hook = ism_change_hook;
Expand All @@ -408,7 +439,12 @@ int ospf_register_opaque_functab(
new->new_lsa_hook = new_lsa_hook;
new->del_lsa_hook = del_lsa_hook;

if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Register Opaque LSA type %u, opaque type %u, (%p)",
__func__, lsa_type, opaque_type, new);

listnode_add(funclist, new);
ospf_opaque_functab_ref(new);

return 0;
}
Expand All @@ -422,17 +458,18 @@ void ospf_delete_opaque_functab(uint8_t lsa_type, uint8_t opaque_type)
if ((funclist = ospf_get_opaque_funclist(lsa_type)) != NULL)
for (ALL_LIST_ELEMENTS(funclist, node, nnode, functab)) {
if (functab->opaque_type == opaque_type) {
/* Cleanup internal control information, if it
* still remains. */
if (functab->oipt != NULL)
free_opaque_info_per_type(functab->oipt,
true);
if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Delete Opaque functab LSA type %u, opaque type %u, (%p)",
__func__, lsa_type,
opaque_type, functab);

/* Dequeue listnode entry from the function table
* list coreesponding to the opaque LSA type.
* Note that the list deletion callback frees
* the functab entry memory.
*/
listnode_delete(funclist, functab);
ospf_opaque_functab_deref(functab);
break;
}
}
Expand Down Expand Up @@ -567,10 +604,15 @@ register_opaque_info_per_type(struct ospf_opaque_functab *functab,
oipt->opaque_type = GET_OPAQUE_TYPE(ntohl(new->data->id.s_addr));
oipt->status = PROC_NORMAL;
oipt->functab = functab;
functab->oipt = oipt;
ospf_opaque_functab_ref(functab);
oipt->id_list = list_new();
oipt->id_list->del = free_opaque_info_per_id;

if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Register Opaque info-per-type LSA type %u, opaque type %u, (%p), Functab (%p)",
__func__, oipt->lsa_type, oipt->opaque_type, oipt,
oipt->functab);

out:
return oipt;
}
Expand Down Expand Up @@ -617,16 +659,14 @@ static void free_opaque_info_per_type(struct opaque_info_per_type *oipt,
listnode_delete(l, oipt);
}

/*
* Delete the function table corresponding to the LSA type and opaque type
* as well. The pointer to the opaque per-type information structure in
* the function table structure be set to NULL to avoid recursion during
* deletion.
*/
if (oipt->functab) {
oipt->functab->oipt = NULL;
ospf_delete_opaque_functab(oipt->lsa_type, oipt->opaque_type);
}
if (oipt->functab)
ospf_opaque_functab_deref(oipt->functab);

if (IS_DEBUG_OSPF_EVENT)
zlog_debug("%s: Free Opaque info-per-type LSA type %u, opaque type %u, (%p), Functab (%p)",
__func__, oipt->lsa_type, oipt->opaque_type, oipt,
oipt->functab);

XFREE(MTYPE_OPAQUE_INFO_PER_TYPE, oipt);
return;
}
Expand Down Expand Up @@ -780,8 +820,9 @@ void ospf_opaque_type9_lsa_if_cleanup(struct ospf_interface *oi)
* While the LSA shouldn't be referenced on any LSA
* lists since the flooding scoped is confined to the
* interface being deleted, clear the pointer to the
* deleted interface for safety's sake after it is
* removed from the area LSDB.
* deleted interface to avoid references and set the
* age to MAXAGE to avoid flush processing when the LSA
* is removed from the interface opaque info list.
*/
if (lsa->oi == oi) {
if (IS_DEBUG_OSPF_EVENT)
Expand All @@ -790,10 +831,10 @@ void ospf_opaque_type9_lsa_if_cleanup(struct ospf_interface *oi)
ntohl(lsa->data->id.s_addr)),
GET_OPAQUE_ID(ntohl(
lsa->data->id.s_addr)));
ospf_lsa_lock(lsa);
ospf_lsdb_delete(lsdb, lsa);
lsa->data->ls_age = htons(OSPF_LSA_MAXAGE);
lsa->oi = NULL;
ospf_lsa_unlock(&lsa);
ospf_lsa_discard(lsa);
}
}

Expand Down

0 comments on commit dec87fa

Please sign in to comment.