Change in osmo-sgsn[master]: sgsn: Fix crash using new libgtp cb_recovery2 API

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/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Tue Jul 24 19:03:40 UTC 2018


Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/10025 )

Change subject: sgsn: Fix crash using new libgtp cb_recovery2 API
......................................................................

sgsn: Fix crash using new libgtp cb_recovery2 API

When PDP CTX CREATE ACK is received with an increased RestartCtr, cb_recovery2
is called first, which will dettach ggsn from al pdp ctx (free the
pdp_t). But when giving control back from the ctrl, libgtp still uses
that freed ctx and sends it back to osmo-sgsn through cb_conf().

As specs state in any case that we need to handle the message containing
the increased RestartCtr as valid, we then need to avoid freeing the pdp
ctx and leave handling for later in cb_conf.

Depends: osmo-ggsn (libgtp) Change-Id I53e92298f2f6b84d662a3300d922e8c2ccb178bc.
Change-Id: I0989c00e18ca95a099e1a312940eaac71957b444
---
M include/osmocom/sgsn/gprs_sgsn.h
M src/gprs/gprs_sgsn.c
M src/gprs/sgsn_libgtp.c
3 files changed, 16 insertions(+), 9 deletions(-)

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



diff --git a/include/osmocom/sgsn/gprs_sgsn.h b/include/osmocom/sgsn/gprs_sgsn.h
index 8eba2d4..6f16dc7 100644
--- a/include/osmocom/sgsn/gprs_sgsn.h
+++ b/include/osmocom/sgsn/gprs_sgsn.h
@@ -362,7 +362,7 @@
 struct sgsn_ggsn_ctx *sgsn_ggsn_ctx_by_addr(struct in_addr *addr);
 struct sgsn_ggsn_ctx *sgsn_ggsn_ctx_find_alloc(uint32_t id);
 void sgsn_ggsn_ctx_drop_pdp(struct sgsn_pdp_ctx *pctx);
-int sgsn_ggsn_ctx_drop_all_pdp(struct sgsn_ggsn_ctx *ggsn);
+int sgsn_ggsn_ctx_drop_all_pdp_except(struct sgsn_ggsn_ctx *ggsn, struct sgsn_pdp_ctx *except);
 void sgsn_ggsn_ctx_add_pdp(struct sgsn_ggsn_ctx *ggc, struct sgsn_pdp_ctx *pdp);
 void sgsn_ggsn_ctx_remove_pdp(struct sgsn_ggsn_ctx *ggc, struct sgsn_pdp_ctx *pdp);
 
diff --git a/src/gprs/gprs_sgsn.c b/src/gprs/gprs_sgsn.c
index e6d88e3..9046157 100644
--- a/src/gprs/gprs_sgsn.c
+++ b/src/gprs/gprs_sgsn.c
@@ -714,13 +714,17 @@
 }
 
 /* High-level function to be called in case a GGSN has disappeared or
- * otherwise lost state (recovery procedure) */
-int sgsn_ggsn_ctx_drop_all_pdp(struct sgsn_ggsn_ctx *ggsn)
+ * otherwise lost state (recovery procedure). It will detach all related pdp ctx
+ * from a ggsn and communicate deact to MS. Optionally (!NULL), one pdp ctx can
+ * be kept alive to allow handling later message which contained the Recovery IE. */
+int sgsn_ggsn_ctx_drop_all_pdp_except(struct sgsn_ggsn_ctx *ggsn, struct sgsn_pdp_ctx *except)
 {
 	int num = 0;
 
 	struct sgsn_pdp_ctx *pdp, *pdp2;
 	llist_for_each_entry_safe(pdp, pdp2, &ggsn->pdp_list, ggsn_list) {
+		if (pdp == except)
+			continue;
 		sgsn_ggsn_ctx_drop_pdp(pdp);
 		num++;
 	}
diff --git a/src/gprs/sgsn_libgtp.c b/src/gprs/sgsn_libgtp.c
index 3813397..7829796 100644
--- a/src/gprs/sgsn_libgtp.c
+++ b/src/gprs/sgsn_libgtp.c
@@ -591,9 +591,10 @@
 }
 
 /* Any message received by GGSN contains a recovery IE */
-static int cb_recovery(struct sockaddr_in *peer, uint8_t recovery)
+static int cb_recovery2(struct sockaddr_in *peer, struct pdp_t *pdp, uint8_t recovery)
 {
 	struct sgsn_ggsn_ctx *ggsn;
+	struct sgsn_pdp_ctx *pctx = NULL;
 
 	ggsn = sgsn_ggsn_ctx_by_addr(&peer->sin_addr);
 	if (!ggsn) {
@@ -606,11 +607,13 @@
 		ggsn->remote_restart_ctr = recovery;
 	} else if (ggsn->remote_restart_ctr != recovery) {
 		/* counter has changed (GGSN restart): release all PDP */
-		LOGP(DGPRS, LOGL_NOTICE, "GGSN recovery (%u->%u), "
-		     "releasing all PDP contexts\n",
-		     ggsn->remote_restart_ctr, recovery);
+		LOGP(DGPRS, LOGL_NOTICE, "GGSN recovery (%u->%u) pdp=%p, "
+		     "releasing all%s PDP contexts\n",
+		     ggsn->remote_restart_ctr, recovery, pdp, pdp ? " other" : "");
 		ggsn->remote_restart_ctr = recovery;
-		sgsn_ggsn_ctx_drop_all_pdp(ggsn);
+		if (pdp)
+			pctx = pdp->priv;
+		sgsn_ggsn_ctx_drop_all_pdp_except(ggsn, pctx);
 	}
 	return 0;
 }
@@ -896,7 +899,7 @@
 	/* Register callbackcs with libgtp */
 	gtp_set_cb_delete_context(gsn, cb_delete_context);
 	gtp_set_cb_conf(gsn, cb_conf);
-	gtp_set_cb_recovery(gsn, cb_recovery);
+	gtp_set_cb_recovery2(gsn, cb_recovery2);
 	gtp_set_cb_data_ind(gsn, cb_data_ind);
 	gtp_set_cb_unsup_ind(gsn, cb_unsup_ind);
 	gtp_set_cb_extheader_ind(gsn, cb_extheader_ind);

-- 
To view, visit https://gerrit.osmocom.org/10025
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0989c00e18ca95a099e1a312940eaac71957b444
Gerrit-Change-Number: 10025
Gerrit-PatchSet: 6
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-CC: Neels Hofmeyr <nhofmeyr at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180724/8b9f252e/attachment.htm>


More information about the gerrit-log mailing list