<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/10025">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">sgsn: Fix crash using new libgtp cb_recovery2 API<br><br>When PDP CTX CREATE ACK is received with an increased RestartCtr, cb_recovery2<br>is called first, which will dettach ggsn from al pdp ctx (free the<br>pdp_t). But when giving control back from the ctrl, libgtp still uses<br>that freed ctx and sends it back to osmo-sgsn through cb_conf().<br><br>As specs state in any case that we need to handle the message containing<br>the increased RestartCtr as valid, we then need to avoid freeing the pdp<br>ctx and leave handling for later in cb_conf.<br><br>Depends: osmo-ggsn (libgtp) Change-Id I53e92298f2f6b84d662a3300d922e8c2ccb178bc.<br>Change-Id: I0989c00e18ca95a099e1a312940eaac71957b444<br>---<br>M include/osmocom/sgsn/gprs_sgsn.h<br>M src/gprs/gprs_sgsn.c<br>M src/gprs/sgsn_libgtp.c<br>3 files changed, 16 insertions(+), 9 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/sgsn/gprs_sgsn.h b/include/osmocom/sgsn/gprs_sgsn.h</span><br><span>index 8eba2d4..6f16dc7 100644</span><br><span>--- a/include/osmocom/sgsn/gprs_sgsn.h</span><br><span>+++ b/include/osmocom/sgsn/gprs_sgsn.h</span><br><span>@@ -362,7 +362,7 @@</span><br><span> struct sgsn_ggsn_ctx *sgsn_ggsn_ctx_by_addr(struct in_addr *addr);</span><br><span> struct sgsn_ggsn_ctx *sgsn_ggsn_ctx_find_alloc(uint32_t id);</span><br><span> void sgsn_ggsn_ctx_drop_pdp(struct sgsn_pdp_ctx *pctx);</span><br><span style="color: hsl(0, 100%, 40%);">-int sgsn_ggsn_ctx_drop_all_pdp(struct sgsn_ggsn_ctx *ggsn);</span><br><span style="color: hsl(120, 100%, 40%);">+int sgsn_ggsn_ctx_drop_all_pdp_except(struct sgsn_ggsn_ctx *ggsn, struct sgsn_pdp_ctx *except);</span><br><span> void sgsn_ggsn_ctx_add_pdp(struct sgsn_ggsn_ctx *ggc, struct sgsn_pdp_ctx *pdp);</span><br><span> void sgsn_ggsn_ctx_remove_pdp(struct sgsn_ggsn_ctx *ggc, struct sgsn_pdp_ctx *pdp);</span><br><span> </span><br><span>diff --git a/src/gprs/gprs_sgsn.c b/src/gprs/gprs_sgsn.c</span><br><span>index e6d88e3..9046157 100644</span><br><span>--- a/src/gprs/gprs_sgsn.c</span><br><span>+++ b/src/gprs/gprs_sgsn.c</span><br><span>@@ -714,13 +714,17 @@</span><br><span> }</span><br><span> </span><br><span> /* High-level function to be called in case a GGSN has disappeared or</span><br><span style="color: hsl(0, 100%, 40%);">- * otherwise lost state (recovery procedure) */</span><br><span style="color: hsl(0, 100%, 40%);">-int sgsn_ggsn_ctx_drop_all_pdp(struct sgsn_ggsn_ctx *ggsn)</span><br><span style="color: hsl(120, 100%, 40%);">+ * otherwise lost state (recovery procedure). It will detach all related pdp ctx</span><br><span style="color: hsl(120, 100%, 40%);">+ * from a ggsn and communicate deact to MS. Optionally (!NULL), one pdp ctx can</span><br><span style="color: hsl(120, 100%, 40%);">+ * be kept alive to allow handling later message which contained the Recovery IE. */</span><br><span style="color: hsl(120, 100%, 40%);">+int sgsn_ggsn_ctx_drop_all_pdp_except(struct sgsn_ggsn_ctx *ggsn, struct sgsn_pdp_ctx *except)</span><br><span> {</span><br><span> int num = 0;</span><br><span> </span><br><span> struct sgsn_pdp_ctx *pdp, *pdp2;</span><br><span> llist_for_each_entry_safe(pdp, pdp2, &ggsn->pdp_list, ggsn_list) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pdp == except)</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span> sgsn_ggsn_ctx_drop_pdp(pdp);</span><br><span> num++;</span><br><span> }</span><br><span>diff --git a/src/gprs/sgsn_libgtp.c b/src/gprs/sgsn_libgtp.c</span><br><span>index 3813397..7829796 100644</span><br><span>--- a/src/gprs/sgsn_libgtp.c</span><br><span>+++ b/src/gprs/sgsn_libgtp.c</span><br><span>@@ -591,9 +591,10 @@</span><br><span> }</span><br><span> </span><br><span> /* Any message received by GGSN contains a recovery IE */</span><br><span style="color: hsl(0, 100%, 40%);">-static int cb_recovery(struct sockaddr_in *peer, uint8_t recovery)</span><br><span style="color: hsl(120, 100%, 40%);">+static int cb_recovery2(struct sockaddr_in *peer, struct pdp_t *pdp, uint8_t recovery)</span><br><span> {</span><br><span> struct sgsn_ggsn_ctx *ggsn;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct sgsn_pdp_ctx *pctx = NULL;</span><br><span> </span><br><span> ggsn = sgsn_ggsn_ctx_by_addr(&peer->sin_addr);</span><br><span> if (!ggsn) {</span><br><span>@@ -606,11 +607,13 @@</span><br><span> ggsn->remote_restart_ctr = recovery;</span><br><span> } else if (ggsn->remote_restart_ctr != recovery) {</span><br><span> /* counter has changed (GGSN restart): release all PDP */</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DGPRS, LOGL_NOTICE, "GGSN recovery (%u->%u), "</span><br><span style="color: hsl(0, 100%, 40%);">- "releasing all PDP contexts\n",</span><br><span style="color: hsl(0, 100%, 40%);">- ggsn->remote_restart_ctr, recovery);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DGPRS, LOGL_NOTICE, "GGSN recovery (%u->%u) pdp=%p, "</span><br><span style="color: hsl(120, 100%, 40%);">+ "releasing all%s PDP contexts\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ ggsn->remote_restart_ctr, recovery, pdp, pdp ? " other" : "");</span><br><span> ggsn->remote_restart_ctr = recovery;</span><br><span style="color: hsl(0, 100%, 40%);">- sgsn_ggsn_ctx_drop_all_pdp(ggsn);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (pdp)</span><br><span style="color: hsl(120, 100%, 40%);">+ pctx = pdp->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+ sgsn_ggsn_ctx_drop_all_pdp_except(ggsn, pctx);</span><br><span> }</span><br><span> return 0;</span><br><span> }</span><br><span>@@ -896,7 +899,7 @@</span><br><span> /* Register callbackcs with libgtp */</span><br><span> gtp_set_cb_delete_context(gsn, cb_delete_context);</span><br><span> gtp_set_cb_conf(gsn, cb_conf);</span><br><span style="color: hsl(0, 100%, 40%);">- gtp_set_cb_recovery(gsn, cb_recovery);</span><br><span style="color: hsl(120, 100%, 40%);">+ gtp_set_cb_recovery2(gsn, cb_recovery2);</span><br><span> gtp_set_cb_data_ind(gsn, cb_data_ind);</span><br><span> gtp_set_cb_unsup_ind(gsn, cb_unsup_ind);</span><br><span> gtp_set_cb_extheader_ind(gsn, cb_extheader_ind);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/10025">change 10025</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/10025"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-sgsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I0989c00e18ca95a099e1a312940eaac71957b444 </div>
<div style="display:none"> Gerrit-Change-Number: 10025 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>