SGSN: Code to help debug / fix sgsn crash in cb_data_ind()

A crash was obsserved in cb_data_ind() when mm is dereferenced.

This patch adds some safeguards that try to prevent the library handle
back-pointer to the pdp_ctx to be NULL, and print a stack backtrace in
case we are free() ing the sgsn-side pdp_ctx while there's still a
library handle attached.

2 files changed, 26 insertions, 1 deletions

diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c
index 0d8fe50ef..84bf512ae 100644
--- a/openbsc/src/gprs/gprs_sgsn.c
+++ b/openbsc/src/gprs/gprs_sgsn.c
@@ -25,6 +25,7 @@
 #include <osmocom/core/talloc.h>
 #include <osmocom/core/timer.h>
 #include <osmocom/core/rate_ctr.h>
+#include <osmocom/core/backtrace.h>
 #include <osmocom/gprs/gprs_ns.h>
 #include <osmocom/gprs/gprs_bssgp.h>
 
@@ -240,12 +241,26 @@ struct sgsn_pdp_ctx *sgsn_pdp_ctx_alloc(struct sgsn_mm_ctx *mm,
 	return pdp;
 }
 
+#include <pdp.h>
 /* you probably want to call sgsn_delete_pdp_ctx() instead */
 void sgsn_pdp_ctx_free(struct sgsn_pdp_ctx *pdp)
 {
 	rate_ctr_group_free(pdp->ctrg);
 	llist_del(&pdp->list);
 	llist_del(&pdp->g_list);
+
+	/* _if_ we still have a library handle, at least set it to NULL
+	 * to avoid any dereferences of the now-deleted PDP context from
+	 * sgsn_libgtp:cb_data_ind() */
+	if (pdp->lib) {
+		struct pdp_t *lib = pdp->lib;
+		LOGP(DGPRS, LOGL_NOTICE, "freeing PDP context that still "
+		     "has a libgtp handle attached to it, this shouldn't "
+		     "happen!\n");
+		osmo_generate_backtrace();
+		lib->priv = NULL;
+	}
+
 	talloc_free(pdp);
 }
 
diff --git a/openbsc/src/gprs/sgsn_libgtp.c b/openbsc/src/gprs/sgsn_libgtp.c
index a19af366e..7c17f9d41 100644
--- a/openbsc/src/gprs/sgsn_libgtp.c
+++ b/openbsc/src/gprs/sgsn_libgtp.c
@@ -317,6 +317,10 @@ static int delete_pdp_conf(struct pdp_t *pdp, void *cbp, int cause)
 	/* Confirm deactivation of PDP context to MS */
 	rc = gsm48_tx_gsm_deact_pdp_acc(pctx);
 
+	/* unlink the now non-existing library handle from the pdp
+	 * context */
+	pctx->lib = NULL;
+
 	sgsn_pdp_ctx_free(pctx);
 
 	return rc;
@@ -422,10 +426,16 @@ static int cb_data_ind(struct pdp_t *lib, void *packet, unsigned int len)
 
 	pdp = lib->priv;
 	if (!pdp) {
-		DEBUGP(DGPRS, "GTP DATA IND from GGSN for unknown PDP\n");
+		LOGP(DGPRS, LOGL_NOTICE,
+		     "GTP DATA IND from GGSN for unknown PDP\n");
 		return -EIO;
 	}
 	mm = pdp->mm;
+	if (!mm) {
+		LOGP(DGPRS, LOGL_ERROR,
+		     "PDP context (imsi=%s) without MM context!\n", mm->imsi);
+		return -EIO;
+	}
 
 	msg = msgb_alloc_headroom(len+256, 128, "GTP->SNDCP");
 	ud = msgb_put(msg, len);