tbf: Fix dangling m_new_tbf pointer

Currently if a 'new' TBF is freed before the 'old' one (where
old_tbf->m_new_tbf == new_tbf), the old_tbf->m_new_tbf is not cleared
and can be accessed later on. This can lead to inconsistencies or
segmentation faults.

This commit adds m_old_tbf which points back from new_tbf to old_pdf.
m_new_tbf and m_old_tbf are either both set to NULL or one is the
reverse pointer of the other (tbf->m_new_tbf->m_old_tbf == tbf and
tbf->m_old_tbf->m_new_tbf == tbf). It extends set_new_tbf and
tbf_free to update the pointee accordingly.

The TBF test is extended to check this invariant at several places.

Sponsored-by: On-Waves ehf

2 files changed, 38 insertions, 0 deletions

diff --git a/src/tbf.cpp b/src/tbf.cpp
index 680a096a..05b61404 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -53,7 +53,22 @@ void gprs_rlcmac_tbf::assign_imsi(const char *imsi)
 
 void gprs_rlcmac_tbf::set_new_tbf(gprs_rlcmac_tbf *tbf)
 {
+	if (m_new_tbf) {
+		if (m_new_tbf == tbf) {
+			LOGP(DRLCMAC, LOGL_NOTICE,
+				"%s reassigning %s to m_new_tbf\n",
+				tbf_name(this), tbf_name(tbf));
+			return;
+		}
+		LOGP(DRLCMAC, LOGL_NOTICE,
+			"%s m_new_tbf is already assigned to %s, "
+			"overwriting the old value with %s\n",
+			tbf_name(this), tbf_name(m_new_tbf), tbf_name(tbf));
+		/* Detach from other TBF */
+		m_new_tbf->m_old_tbf = NULL;
+	}
 	m_new_tbf = tbf;
+	tbf->m_old_tbf = this;
 }
 
 gprs_rlcmac_ul_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
@@ -138,6 +153,28 @@ void tbf_free(struct gprs_rlcmac_tbf *tbf)
 	else
 		tbf->bts->tbf_dl_freed();
 
+	if (tbf->m_old_tbf) {
+		LOGP(DRLCMAC, LOGL_INFO, "%s Old TBF %s still exists, detaching\n",
+			tbf_name(tbf), tbf_name(tbf->m_old_tbf));
+		if (tbf->m_old_tbf->m_new_tbf == tbf)
+			tbf->m_old_tbf->m_new_tbf = NULL;
+		else
+			LOGP(DRLCMAC, LOGL_ERROR, "%s Software error: "
+				"tbf->m_old_tbf->m_new_tbf != tbf\n",
+				tbf_name(tbf));
+	}
+
+	if (tbf->m_new_tbf) {
+		LOGP(DRLCMAC, LOGL_INFO, "%s New TBF %s still exists, detaching\n",
+			tbf_name(tbf), tbf_name(tbf->m_new_tbf));
+		if (tbf->m_new_tbf->m_old_tbf == tbf)
+			tbf->m_new_tbf->m_old_tbf = NULL;
+		else
+			LOGP(DRLCMAC, LOGL_ERROR, "%s Software error: "
+				"tbf->m_new_tbf->m_old_tbf != tbf\n",
+				tbf_name(tbf));
+	}
+
 	LOGP(DRLCMAC, LOGL_DEBUG, "********** TBF ends here **********\n");
 	talloc_free(tbf);
 }
diff --git a/src/tbf.h b/src/tbf.h
index 69f1f053..1bea31d9 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -169,6 +169,7 @@ struct gprs_rlcmac_tbf {
 	enum gprs_rlcmac_tbf_ul_ack_state ul_ack_state;
 
 	gprs_rlcmac_tbf *m_new_tbf;
+	gprs_rlcmac_tbf *m_old_tbf; /* reverse pointer for m_new_tbf */
 
 	enum gprs_rlcmac_tbf_poll_state poll_state;
 	uint32_t poll_fn; /* frame number to poll */