[PATCH] openbsc[master]: sgsn: Fix deeply flawed copying logic for PDP context activa...

[Holger Freyther]

Review at  https://gerrit.osmocom.org/1686

sgsn: Fix deeply flawed copying logic for PDP context activation

It is one of these changes that should have never worked but did
for a long time. Only recently a corrupted GTP message was seen.
The code in ccd2312d10e14747e8a4d26d8f72b052ffcfc282 tried to
solve the right problem but was deeply flawed.

* Make the code operate on the copied message and not the original
one that is deleted by the underlaying layers on return
* Add an out variable to determine if the msgb should be deleted
and assume that by default it will be deleted.

Change-Id: I564526e7cde2b8a2f0ce900492cd38fc23c176a7
---
M openbsc/src/gprs/gprs_gmm.c
1 file changed, 9 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/86/1686/1

diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index 88be512..6471c2e 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -2169,6 +2169,7 @@
 
 	/* The context is gone while we made a request */
 	if (!lookup->mmctx) {
+		talloc_free(lookup->orig_msg);
 		talloc_free(lookup);
 		return;
 	}
@@ -2230,6 +2231,7 @@
 			lookup->sapi, &lookup->tp, 1);
 
 	/* Now free it */
+	talloc_free(lookup->orig_msg);
 	talloc_free(lookup);
 	return;
 
@@ -2237,10 +2239,11 @@
 	gsm48_tx_gsm_act_pdp_rej(lookup->mmctx, lookup->ti,
 				GMM_CAUSE_NET_FAIL, 0, NULL);
 	lookup->mmctx->ggsn_lookup = NULL;
+	talloc_free(lookup->orig_msg);
 	talloc_free(lookup);
 }
 
-static int do_act_pdp_req(struct sgsn_mm_ctx *mmctx, struct msgb *msg)
+static int do_act_pdp_req(struct sgsn_mm_ctx *mmctx, struct msgb *msg, int *delete)
 {
 	struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(msg);
 	struct gsm48_act_pdp_ctx_req *act_req = (struct gsm48_act_pdp_ctx_req *) gh->data;
@@ -2389,6 +2392,7 @@
 		LOGMMCTXP(LOGL_ERROR, mmctx, "Failed to start ares query.\n");
 		goto no_context;
 	}
+	*delete = 0;
 
 	return 0;
 
@@ -2402,6 +2406,7 @@
 static int gsm48_rx_gsm_act_pdp_req(struct sgsn_mm_ctx *mmctx,
 				    struct msgb *_msg)
 {
+	int delete = 1;
 	struct msgb *msg;
 	int rc;
 
@@ -2428,8 +2433,9 @@
 						0, NULL);
 	}
 
-	rc = do_act_pdp_req(mmctx, _msg);
-	msgb_free(msg);
+	rc = do_act_pdp_req(mmctx, msg, &delete);
+	if (delete)
+		msgb_free(msg);
 	return rc;
 }
 

-- 
To view, visit https://gerrit.osmocom.org/1686
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I564526e7cde2b8a2f0ce900492cd38fc23c176a7
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther <holger at freyther.de>