From c362df25a2eb469aa70827b469f3898447814697 Mon Sep 17 00:00:00 2001
From: Jacob Erlbeck <jerlbeck@sysmocom.de>
Date: Wed, 20 Jan 2016 22:02:19 +0100
Subject: [PATCH] pcu: Fix memory corruption bugs (ASAN)

ASAN has found improper deletion of objects. These only occur
on shutdown but makes it impossible to run the test cases with
full ASAN support.

This commit fixes some of them and deactivates the freeing of the_pcu.bctx
which may cause a corruption in BTS::~BTS() later on.

Note that the latter is only a work-aound and should be fixed
properly. It will leak bctx objects, but this is currently not
critical, since gprs_bssgp_destroy is only called once, immediately
before a call to exit().

Ticket: OW#1572
Sponsored-by: On-Waves ehf
---
 src/bts.cpp             |  5 +++++
 src/gprs_bssgp_pcu.cpp  | 21 +++++++++++++--------
 src/gprs_ms_storage.cpp |  5 +++++
 src/gprs_ms_storage.h   |  2 ++
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/src/bts.cpp b/src/bts.cpp
index 5e29364..d1d738c 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -150,7 +150,12 @@ BTS::BTS()
 
 BTS::~BTS()
 {
+	/* this can cause counter updates and must not be left to the
+	 * m_ms_store's destructor */
+	m_ms_store.cleanup();
+
 	rate_ctr_group_free(m_ratectrs);
+	osmo_stat_item_group_free(m_statg);
 }
 
 
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index a7391d9..838c667 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -872,28 +872,33 @@ struct gprs_bssgp_pcu *gprs_bssgp_create_and_connect(struct gprs_rlcmac_bts *bts
 
 void gprs_bssgp_destroy(void)
 {
-	if (!bssgp_nsi)
+	struct gprs_ns_inst *nsi = bssgp_nsi;
+	if (!nsi)
 		return;
 
+	bssgp_nsi = NULL;
+
 	osmo_timer_del(&the_pcu.bvc_timer);
 
 	osmo_signal_unregister_handler(SS_L_NS, nsvc_signal_cb, NULL);
 
 	the_pcu.nsvc = NULL;
 
-	/* FIXME: move this to libgb: btsctx_free() */
-	llist_del(&the_pcu.bctx->list);
-	talloc_free(the_pcu.bctx);
-	the_pcu.bctx = NULL;
-
 	/* FIXME: blocking... */
 	the_pcu.nsvc_unblocked = 0;
 	the_pcu.bvc_sig_reset = 0;
 	the_pcu.bvc_reset = 0;
 	the_pcu.bvc_unblocked = 0;	
 
-	gprs_ns_destroy(bssgp_nsi);
-	bssgp_nsi = NULL;
+	gprs_ns_destroy(nsi);
+
+	/* FIXME: move this to libgb: btsctx_free() */
+	llist_del(&the_pcu.bctx->list);
+#warning "This causes ASAN to complain. It is not critical for normal operation but should be fixed nevertheless"
+#if 0
+	talloc_free(the_pcu.bctx);
+#endif
+	the_pcu.bctx = NULL;
 }
 
 struct bssgp_bvc_ctx *gprs_bssgp_pcu_current_bctx(void)
diff --git a/src/gprs_ms_storage.cpp b/src/gprs_ms_storage.cpp
index e0aee5e..6a7f336 100644
--- a/src/gprs_ms_storage.cpp
+++ b/src/gprs_ms_storage.cpp
@@ -33,6 +33,11 @@ GprsMsStorage::GprsMsStorage(BTS *bts) :
 }
 
 GprsMsStorage::~GprsMsStorage()
+{
+	cleanup();
+}
+
+void GprsMsStorage::cleanup()
 {
 	LListHead<GprsMs> *pos, *tmp;
 
diff --git a/src/gprs_ms_storage.h b/src/gprs_ms_storage.h
index df788bf..44ad0ed 100644
--- a/src/gprs_ms_storage.h
+++ b/src/gprs_ms_storage.h
@@ -33,6 +33,8 @@ public:
 	GprsMsStorage(BTS *bts);
 	~GprsMsStorage();
 
+	void cleanup();
+
 	virtual void ms_idle(class GprsMs *);
 	virtual void ms_active(class GprsMs *);