From 47cd634819cc3b218a9ae4911d5650fe26dee2e7 Mon Sep 17 00:00:00 2001 From: Manoj Naragund Date: Mon, 19 Dec 2022 04:07:22 -0800 Subject: [PATCH] ospf6d: Fixing memory leak in ospf6_lsa_create_headeronly for both master and slave. Problem Statement: ================= Memory leak backtraces 2022-11-23 01:51:10,525 - ERROR: ==842== 1,100 (1,000 direct, 100 indirect) bytes in 5 blocks are definitely lost in loss record 29 of 31 2022-11-23 01:51:10,525 - ERROR: ==842== at 0x4C31FAC: calloc (vg_replace_malloc.c:762) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x4E8A1BF: qcalloc (memory.c:111) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13555A: ospf6_lsa_alloc (ospf6_lsa.c:723) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x1355F3: ospf6_lsa_create_headeronly (ospf6_lsa.c:756) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x135702: ospf6_lsa_copy (ospf6_lsa.c:790) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13B64B: ospf6_dbdesc_recv_slave (ospf6_message.c:976) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13B64B: ospf6_dbdesc_recv (ospf6_message.c:1038) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13B64B: ospf6_read_helper (ospf6_message.c:1838) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13B64B: ospf6_receive (ospf6_message.c:1875) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x4EB741B: thread_call (thread.c:1692) 2022-11-23 01:51:10,526 - ERROR: ==842== by 0x4E85B17: frr_run (libfrr.c:1068) 2022-11-23 01:51:10,526 - ERROR: ==842== by 0x119585: main (ospf6_main.c:228) 2022-11-23 01:51:10,526 - ERROR: ==842== 2022-11-23 01:51:10,524 - ERROR: Found memory leak in module ospf6d 2022-11-23 01:51:10,525 - ERROR: ==842== 220 (200 direct, 20 indirect) bytes in 1 blocks are definitely lost in loss record 21 of 31 2022-11-23 01:51:10,525 - ERROR: ==842== at 0x4C31FAC: calloc (vg_replace_malloc.c:762) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x4E8A1BF: qcalloc (memory.c:111) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13555A: ospf6_lsa_alloc (ospf6_lsa.c:723) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x1355F3: ospf6_lsa_create_headeronly (ospf6_lsa.c:756) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x135702: ospf6_lsa_copy (ospf6_lsa.c:790) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13BBCE: ospf6_dbdesc_recv_master (ospf6_message.c:760) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13BBCE: ospf6_dbdesc_recv (ospf6_message.c:1036) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13BBCE: ospf6_read_helper (ospf6_message.c:1838) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x13BBCE: ospf6_receive (ospf6_message.c:1875) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x4EB741B: thread_call (thread.c:1692) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x4E85B17: frr_run (libfrr.c:1068) 2022-11-23 01:51:10,525 - ERROR: ==842== by 0x119585: main (ospf6_main.c:228) 2022-11-23 01:51:10,525 - ERROR: ==842== RCA: ==== These memory leaks are beacuse of last lsa in neighbour's request_list is not getting freed beacuse of lsa lock. The last request has an addtional lock which is added as a part of ospf6_make_lsreq, this lock needs to be removed in order for the lsa to get freed. Fix: ==== Check and remove the lock on the last request in all the functions. Signed-off-by: Manoj Naragund --- ospf6d/ospf6_neighbor.c | 65 +++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 42 deletions(-) diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c index 439b94c9af..a4ba1fb569 100644 --- a/ospf6d/ospf6_neighbor.c +++ b/ospf6d/ospf6_neighbor.c @@ -106,6 +106,23 @@ struct ospf6_neighbor *ospf6_area_neighbor_lookup(struct ospf6_area *area, return NULL; } +static void ospf6_neighbor_clear_ls_lists(struct ospf6_neighbor *on) +{ + struct ospf6_lsa *lsa; + struct ospf6_lsa *lsanext; + + ospf6_lsdb_remove_all(on->summary_list); + if (on->last_ls_req) { + ospf6_lsa_unlock(on->last_ls_req); + on->last_ls_req = NULL; + } + ospf6_lsdb_remove_all(on->request_list); + for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { + ospf6_decrement_retrans_count(lsa); + ospf6_lsdb_remove(lsa, on->retrans_list); + } +} + /* create ospf6_neighbor */ struct ospf6_neighbor *ospf6_neighbor_create(uint32_t router_id, struct ospf6_interface *oi) @@ -147,14 +164,7 @@ struct ospf6_neighbor *ospf6_neighbor_create(uint32_t router_id, void ospf6_neighbor_delete(struct ospf6_neighbor *on) { - struct ospf6_lsa *lsa, *lsanext; - - ospf6_lsdb_remove_all(on->summary_list); - ospf6_lsdb_remove_all(on->request_list); - for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { - ospf6_decrement_retrans_count(lsa); - ospf6_lsdb_remove(lsa, on->retrans_list); - } + ospf6_neighbor_clear_ls_lists(on); ospf6_lsdb_remove_all(on->dbdesc_list); ospf6_lsdb_remove_all(on->lsupdate_list); @@ -339,12 +349,7 @@ void negotiation_done(struct thread *thread) zlog_debug("Neighbor Event %s: *NegotiationDone*", on->name); /* clear ls-list */ - ospf6_lsdb_remove_all(on->summary_list); - ospf6_lsdb_remove_all(on->request_list); - for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { - ospf6_decrement_retrans_count(lsa); - ospf6_lsdb_remove(lsa, on->retrans_list); - } + ospf6_neighbor_clear_ls_lists(on); /* Interface scoped LSAs */ for (ALL_LSDB(on->ospf6_if->lsdb, lsa, lsanext)) { @@ -464,7 +469,6 @@ void loading_done(struct thread *thread) void adj_ok(struct thread *thread) { struct ospf6_neighbor *on; - struct ospf6_lsa *lsa, *lsanext; on = (struct ospf6_neighbor *)THREAD_ARG(thread); assert(on); @@ -486,19 +490,13 @@ void adj_ok(struct thread *thread) } else if (on->state >= OSPF6_NEIGHBOR_EXSTART && !need_adjacency(on)) { ospf6_neighbor_state_change(OSPF6_NEIGHBOR_TWOWAY, on, OSPF6_NEIGHBOR_EVENT_ADJ_OK); - ospf6_lsdb_remove_all(on->summary_list); - ospf6_lsdb_remove_all(on->request_list); - for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { - ospf6_decrement_retrans_count(lsa); - ospf6_lsdb_remove(lsa, on->retrans_list); - } + ospf6_neighbor_clear_ls_lists(on); } } void seqnumber_mismatch(struct thread *thread) { struct ospf6_neighbor *on; - struct ospf6_lsa *lsa, *lsanext; on = (struct ospf6_neighbor *)THREAD_ARG(thread); assert(on); @@ -515,12 +513,7 @@ void seqnumber_mismatch(struct thread *thread) SET_FLAG(on->dbdesc_bits, OSPF6_DBDESC_MBIT); SET_FLAG(on->dbdesc_bits, OSPF6_DBDESC_IBIT); - ospf6_lsdb_remove_all(on->summary_list); - ospf6_lsdb_remove_all(on->request_list); - for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { - ospf6_decrement_retrans_count(lsa); - ospf6_lsdb_remove(lsa, on->retrans_list); - } + ospf6_neighbor_clear_ls_lists(on); THREAD_OFF(on->thread_send_dbdesc); on->dbdesc_seqnum++; /* Incr seqnum as per RFC2328, sec 10.3 */ @@ -532,7 +525,6 @@ void seqnumber_mismatch(struct thread *thread) void bad_lsreq(struct thread *thread) { struct ospf6_neighbor *on; - struct ospf6_lsa *lsa, *lsanext; on = (struct ospf6_neighbor *)THREAD_ARG(thread); assert(on); @@ -549,12 +541,7 @@ void bad_lsreq(struct thread *thread) SET_FLAG(on->dbdesc_bits, OSPF6_DBDESC_MBIT); SET_FLAG(on->dbdesc_bits, OSPF6_DBDESC_IBIT); - ospf6_lsdb_remove_all(on->summary_list); - ospf6_lsdb_remove_all(on->request_list); - for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { - ospf6_decrement_retrans_count(lsa); - ospf6_lsdb_remove(lsa, on->retrans_list); - } + ospf6_neighbor_clear_ls_lists(on); THREAD_OFF(on->thread_send_dbdesc); on->dbdesc_seqnum++; /* Incr seqnum as per RFC2328, sec 10.3 */ @@ -567,7 +554,6 @@ void bad_lsreq(struct thread *thread) void oneway_received(struct thread *thread) { struct ospf6_neighbor *on; - struct ospf6_lsa *lsa, *lsanext; on = (struct ospf6_neighbor *)THREAD_ARG(thread); assert(on); @@ -582,12 +568,7 @@ void oneway_received(struct thread *thread) OSPF6_NEIGHBOR_EVENT_ONEWAY_RCVD); thread_add_event(master, neighbor_change, on->ospf6_if, 0, NULL); - ospf6_lsdb_remove_all(on->summary_list); - ospf6_lsdb_remove_all(on->request_list); - for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { - ospf6_decrement_retrans_count(lsa); - ospf6_lsdb_remove(lsa, on->retrans_list); - } + ospf6_neighbor_clear_ls_lists(on); THREAD_OFF(on->thread_send_dbdesc); THREAD_OFF(on->thread_send_lsreq);