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

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Holger Freyther gerrit-no-reply at lists.osmocom.org
Wed Feb 1 15:02:55 UTC 2017


Holger Freyther has submitted this change and it was merged.

Change subject: sgsn: Fix deeply flawed copying logic for PDP context activation
......................................................................


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(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index dc73693..6fde757 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, bool *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)
 {
+	bool 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: merged
Gerrit-Change-Id: I564526e7cde2b8a2f0ce900492cd38fc23c176a7
Gerrit-PatchSet: 3
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list