ospf6d: Prevent use after free

the for (ALL_LSDB...) macro was iterating over lsa,
when lsa had just been freed in these functions.
Remove the macro and make the adjustments saving lsa_next
before the free.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

3 files changed, 64 insertions, 26 deletions

diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c
index b551dbdfa6..0a9f1c6f7c 100644
--- a/ospf6d/ospf6_lsdb.c
+++ b/ospf6d/ospf6_lsdb.c
@@ -298,13 +298,17 @@ struct ospf6_lsa *ospf6_lsdb_next(const struct route_node *iterend,
 
 void ospf6_lsdb_remove_all(struct ospf6_lsdb *lsdb)
 {
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 
 	if (lsdb == NULL)
 		return;
 
-	for (ALL_LSDB(lsdb, lsa))
+	for (iterend = ospf6_lsdb_head(lsdb, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		ospf6_lsdb_remove(lsa, lsdb);
+	}
 }
 
 void ospf6_lsdb_lsa_unlock(struct ospf6_lsa *lsa)
@@ -319,9 +323,12 @@ void ospf6_lsdb_lsa_unlock(struct ospf6_lsa *lsa)
 int ospf6_lsdb_maxage_remover(struct ospf6_lsdb *lsdb)
 {
 	int reschedule = 0;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 
-	for (ALL_LSDB(lsdb, lsa)) {
+	for (iterend = ospf6_lsdb_head(lsdb, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		if (!OSPF6_LSA_IS_MAXAGE(lsa))
 			continue;
 		if (lsa->retrans_count != 0) {
diff --git a/ospf6d/ospf6_message.c b/ospf6d/ospf6_message.c
index 4acb5e3b2e..da42a24252 100644
--- a/ospf6d/ospf6_message.c
+++ b/ospf6d/ospf6_message.c
@@ -1866,7 +1866,8 @@ int ospf6_dbdesc_send(struct thread *thread)
 int ospf6_dbdesc_send_newone(struct thread *thread)
 {
 	struct ospf6_neighbor *on;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 	unsigned int size = 0;
 
 	on = (struct ospf6_neighbor *)THREAD_ARG(thread);
@@ -1876,7 +1877,10 @@ int ospf6_dbdesc_send_newone(struct thread *thread)
 	   structure)
 	   so that ospf6_send_dbdesc () can send those LSAs */
 	size = sizeof(struct ospf6_lsa_header) + sizeof(struct ospf6_dbdesc);
-	for (ALL_LSDB(on->summary_list, lsa)) {
+
+	for (iterend = ospf6_lsdb_head(on->summary_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		if (size + sizeof(struct ospf6_lsa_header)
 		    > ospf6_packet_max(on->ospf6_if)) {
 			ospf6_lsdb_lsa_unlock(lsa);
@@ -2019,7 +2023,8 @@ int ospf6_lsupdate_send_neighbor(struct thread *thread)
 	struct ospf6_lsupdate *lsupdate;
 	uint8_t *p;
 	int lsa_cnt;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 
 	on = (struct ospf6_neighbor *)THREAD_ARG(thread);
 	on->thread_send_lsupdate = (struct thread *)NULL;
@@ -2044,7 +2049,9 @@ int ospf6_lsupdate_send_neighbor(struct thread *thread)
 
 	/* lsupdate_list lists those LSA which doesn't need to be
 	   retransmitted. remove those from the list */
-	for (ALL_LSDB(on->lsupdate_list, lsa)) {
+	for (iterend = ospf6_lsdb_head(on->lsupdate_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		/* MTU check */
 		if ((p - sendbuf + (unsigned int)OSPF6_LSA_SIZE(lsa->header))
 		    > ospf6_packet_max(on->ospf6_if)) {
@@ -2074,7 +2081,7 @@ int ospf6_lsupdate_send_neighbor(struct thread *thread)
 		p += OSPF6_LSA_SIZE(lsa->header);
 		lsa_cnt++;
 
-		assert(lsa->lock == 2);
+		assert(lsa->lock == 1);
 		ospf6_lsdb_remove(lsa, on->lsupdate_list);
 	}
 
@@ -2202,7 +2209,8 @@ int ospf6_lsupdate_send_interface(struct thread *thread)
 	struct ospf6_lsupdate *lsupdate;
 	uint8_t *p;
 	int lsa_cnt;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 
 	oi = (struct ospf6_interface *)THREAD_ARG(thread);
 	oi->thread_send_lsupdate = (struct thread *)NULL;
@@ -2228,7 +2236,9 @@ int ospf6_lsupdate_send_interface(struct thread *thread)
 	p = (uint8_t *)((caddr_t)lsupdate + sizeof(struct ospf6_lsupdate));
 	lsa_cnt = 0;
 
-	for (ALL_LSDB(oi->lsupdate_list, lsa)) {
+	for (iterend = ospf6_lsdb_head(oi->lsupdate_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		/* MTU check */
 		if ((p - sendbuf + ((unsigned int)OSPF6_LSA_SIZE(lsa->header)))
 		    > ospf6_packet_max(oi)) {
@@ -2263,7 +2273,7 @@ int ospf6_lsupdate_send_interface(struct thread *thread)
 		p += OSPF6_LSA_SIZE(lsa->header);
 		lsa_cnt++;
 
-		assert(lsa->lock == 2);
+		assert(lsa->lock == 1);
 		ospf6_lsdb_remove(lsa, oi->lsupdate_list);
 	}
 
@@ -2289,7 +2299,8 @@ int ospf6_lsack_send_neighbor(struct thread *thread)
 	struct ospf6_neighbor *on;
 	struct ospf6_header *oh;
 	uint8_t *p;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 	int lsa_cnt = 0;
 
 	on = (struct ospf6_neighbor *)THREAD_ARG(thread);
@@ -2312,7 +2323,9 @@ int ospf6_lsack_send_neighbor(struct thread *thread)
 
 	p = (uint8_t *)((caddr_t)oh + sizeof(struct ospf6_header));
 
-	for (ALL_LSDB(on->lsack_list, lsa)) {
+	for (iterend = ospf6_lsdb_head(on->lsack_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		/* MTU check */
 		if (p - sendbuf + sizeof(struct ospf6_lsa_header)
 		    > ospf6_packet_max(on->ospf6_if)) {
@@ -2340,7 +2353,7 @@ int ospf6_lsack_send_neighbor(struct thread *thread)
 		memcpy(p, lsa->header, sizeof(struct ospf6_lsa_header));
 		p += sizeof(struct ospf6_lsa_header);
 
-		assert(lsa->lock == 2);
+		assert(lsa->lock == 1);
 		ospf6_lsdb_remove(lsa, on->lsack_list);
 		lsa_cnt++;
 	}
@@ -2367,7 +2380,8 @@ int ospf6_lsack_send_interface(struct thread *thread)
 	struct ospf6_interface *oi;
 	struct ospf6_header *oh;
 	uint8_t *p;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 	int lsa_cnt = 0;
 
 	oi = (struct ospf6_interface *)THREAD_ARG(thread);
@@ -2391,7 +2405,9 @@ int ospf6_lsack_send_interface(struct thread *thread)
 
 	p = (uint8_t *)((caddr_t)oh + sizeof(struct ospf6_header));
 
-	for (ALL_LSDB(oi->lsack_list, lsa)) {
+	for (iterend = ospf6_lsdb_head(oi->lsack_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		/* MTU check */
 		if (p - sendbuf + sizeof(struct ospf6_lsa_header)
 		    > ospf6_packet_max(oi)) {
@@ -2409,7 +2425,7 @@ int ospf6_lsack_send_interface(struct thread *thread)
 		memcpy(p, lsa->header, sizeof(struct ospf6_lsa_header));
 		p += sizeof(struct ospf6_lsa_header);
 
-		assert(lsa->lock == 2);
+		assert(lsa->lock == 1);
 		ospf6_lsdb_remove(lsa, oi->lsack_list);
 		lsa_cnt++;
 	}
diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c
index dccf15aee2..4318db5225 100644
--- a/ospf6d/ospf6_neighbor.c
+++ b/ospf6d/ospf6_neighbor.c
@@ -112,11 +112,15 @@ struct ospf6_neighbor *ospf6_neighbor_create(uint32_t router_id,
 
 void ospf6_neighbor_delete(struct ospf6_neighbor *on)
 {
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 
 	ospf6_lsdb_remove_all(on->summary_list);
 	ospf6_lsdb_remove_all(on->request_list);
-	for (ALL_LSDB(on->retrans_list, lsa)) {
+
+	for (iterend = ospf6_lsdb_head(on->retrans_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		ospf6_decrement_retrans_count(lsa);
 		ospf6_lsdb_remove(lsa, on->retrans_list);
 	}
@@ -287,7 +291,8 @@ int twoway_received(struct thread *thread)
 int negotiation_done(struct thread *thread)
 {
 	struct ospf6_neighbor *on;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 
 	on = (struct ospf6_neighbor *)THREAD_ARG(thread);
 	assert(on);
@@ -301,7 +306,10 @@ int negotiation_done(struct thread *thread)
 	/* clear ls-list */
 	ospf6_lsdb_remove_all(on->summary_list);
 	ospf6_lsdb_remove_all(on->request_list);
-	for (ALL_LSDB(on->retrans_list, lsa)) {
+
+	for (iterend = ospf6_lsdb_head(on->retrans_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		ospf6_decrement_retrans_count(lsa);
 		ospf6_lsdb_remove(lsa, on->retrans_list);
 	}
@@ -495,7 +503,8 @@ int seqnumber_mismatch(struct thread *thread)
 int bad_lsreq(struct thread *thread)
 {
 	struct ospf6_neighbor *on;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 
 	on = (struct ospf6_neighbor *)THREAD_ARG(thread);
 	assert(on);
@@ -514,7 +523,10 @@ int bad_lsreq(struct thread *thread)
 
 	ospf6_lsdb_remove_all(on->summary_list);
 	ospf6_lsdb_remove_all(on->request_list);
-	for (ALL_LSDB(on->retrans_list, lsa)) {
+
+	for (iterend = ospf6_lsdb_head(on->retrans_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		ospf6_decrement_retrans_count(lsa);
 		ospf6_lsdb_remove(lsa, on->retrans_list);
 	}
@@ -532,7 +544,8 @@ int bad_lsreq(struct thread *thread)
 int oneway_received(struct thread *thread)
 {
 	struct ospf6_neighbor *on;
-	struct ospf6_lsa *lsa;
+	struct ospf6_lsa *lsa, *lsa_next;
+	const struct route_node *iterend;
 
 	on = (struct ospf6_neighbor *)THREAD_ARG(thread);
 	assert(on);
@@ -549,7 +562,9 @@ int oneway_received(struct thread *thread)
 
 	ospf6_lsdb_remove_all(on->summary_list);
 	ospf6_lsdb_remove_all(on->request_list);
-	for (ALL_LSDB(on->retrans_list, lsa)) {
+	for (iterend = ospf6_lsdb_head(on->retrans_list, 0, 0, 0, &lsa); lsa;
+	     lsa = lsa_next) {
+		lsa_next = ospf6_lsdb_next(iterend, lsa);
 		ospf6_decrement_retrans_count(lsa);
 		ospf6_lsdb_remove(lsa, on->retrans_list);
 	}