summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJacob Erlbeck <jerlbeck@sysmocom.de>2014-10-13 10:32:00 +0200
committerHolger Hans Peter Freyther <holger@moiji-mobile.com>2014-10-27 10:25:13 +0100
commit99985b5ea8e2d69d1e63a9423fbe40b872b0c0f5 (patch)
tree06257bc6fdf1bfbc1f260fdf2e54e448746eba52
parentae20b4b31b679d7ea057f4db8427b99293068ec5 (diff)
sgsn: Delete PDP contexts properly
Currently the PDP contexts are hard freed (via sgsn_pdp_ctx_free) at some places in gprs_gmm.c on the reception of a Detach Req and on re-use of an IMSI that is already associated with an MM context. This can lead to segfaults when there is a pending request or a data indication at libgtp. This patch add a new function sgsn_pdp_ctx_terminate that de-associates the PTP context from the MM context, deactivates SNDCP, sets pdp->mm to NULL and then calls sgsn_delete_pdp_ctx. sgsn_libgtp is updated to check for pdp->mm being non-NULL before dereferencing it. The sgsn_pdp_ctx_terminate function will be called for each PDP context of an MM context before this context is going to be deleted via sgsn_mm_ctx_free. To ensure, that the ctx->llme (which is accessed during the deactivation of SNDCP) remains valid, the call to gprs_llgmm_assign is moved after the call to sgsn_mm_ctx_free. The handling of re-used IMSIs is changed to mimic the processing of a Detach Req. Addresses: <0002> gprs_gmm.c:654 MM(/f6b31ab0) Deleting old MM Context for same IMSI p_tmsi_old=0xc6f19134 <000f> gprs_sgsn.c:259 PDP freeing PDP context that still has a libgtp handle attached to it, this shouldn't happen! [...] SEGFAULT Ticket: OW#1311 Sponsored-by: On-Waves ehf
-rw-r--r--openbsc/include/openbsc/gprs_sgsn.h4
-rw-r--r--openbsc/src/gprs/gprs_gmm.c54
-rw-r--r--openbsc/src/gprs/gprs_sgsn.c34
-rw-r--r--openbsc/src/gprs/sgsn_libgtp.c22
-rw-r--r--openbsc/src/gprs/sgsn_vty.c3
5 files changed, 84 insertions, 33 deletions
diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index a04ab91c2..c9df82444 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -180,7 +180,8 @@ struct sgsn_pdp_ctx {
};
#define LOGPDPCTXP(level, pdp, fmt, args...) \
- LOGP(DGPRS, level, "PDP(%s/%u) " fmt, (pdp)->mm->imsi, (pdp)->ti, ## args)
+ LOGP(DGPRS, level, "PDP(%s/%u) " \
+ fmt, (pdp)->mm ? (pdp)->mm->imsi : "---", (pdp)->ti, ## args)
/* look up PDP context by MM context and NSAPI */
struct sgsn_pdp_ctx *sgsn_pdp_ctx_by_nsapi(const struct sgsn_mm_ctx *mm,
@@ -191,6 +192,7 @@ struct sgsn_pdp_ctx *sgsn_pdp_ctx_by_tid(const struct sgsn_mm_ctx *mm,
struct sgsn_pdp_ctx *sgsn_pdp_ctx_alloc(struct sgsn_mm_ctx *mm,
uint8_t nsapi);
+void sgsn_pdp_ctx_terminate(struct sgsn_pdp_ctx *pdp);
void sgsn_pdp_ctx_free(struct sgsn_pdp_ctx *pdp);
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index a2d61cfe9..9df0cc62f 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -265,6 +265,28 @@ static void mmctx2msgid(struct msgb *msg, const struct sgsn_mm_ctx *mm)
msgb_nsei(msg) = mm->nsei;
}
+static void mm_ctx_cleanup_free(struct sgsn_mm_ctx *ctx, const char *log_text)
+{
+ struct gprs_llc_llme *llme = ctx->llme;
+ uint32_t tlli = ctx->tlli;
+ struct sgsn_pdp_ctx *pdp, *pdp2;
+
+ /* Mark MM state as deregistered */
+ ctx->mm_state = GMM_DEREGISTERED;
+
+ llist_for_each_entry_safe(pdp, pdp2, &ctx->pdp_list, list) {
+ LOGMMCTXP(LOGL_NOTICE, ctx, "Dropping PDP context for NSAPI=%u "
+ "due to %s\n", pdp->nsapi, log_text);
+ sgsn_pdp_ctx_terminate(pdp);
+ }
+
+ sgsn_mm_ctx_free(ctx);
+ ctx = NULL;
+
+ /* TLLI unassignment, must be called after sgsn_mm_ctx_free */
+ gprs_llgmm_assign(llme, tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
+}
+
/* Chapter 9.4.18 */
static int _tx_status(struct msgb *msg, uint8_t cause,
struct sgsn_mm_ctx *mmctx, int sm)
@@ -603,15 +625,16 @@ static int gsm48_rx_gmm_id_resp(struct sgsn_mm_ctx *ctx, struct msgb *msg)
struct sgsn_mm_ctx *ictx;
ictx = sgsn_mm_ctx_by_imsi(mi_string);
if (ictx) {
+ /* Handle it like in gsm48_rx_gmm_det_req,
+ * except that no messages are sent to the BSS */
+
LOGMMCTXP(LOGL_NOTICE, ctx, "Deleting old MM Context for same IMSI "
"p_tmsi_old=0x%08x\n",
ictx->p_tmsi);
- gprs_llgmm_assign(ictx->llme, ictx->tlli,
- 0xffffffff, GPRS_ALGO_GEA0, NULL);
- /* FIXME: this is a hard free, we don't
- * clean-up any PDP contexts on the
- * libgtp side */
- sgsn_mm_ctx_free(ictx);
+
+ ictx->mm_state = GMM_DEREGISTERED;
+
+ mm_ctx_cleanup_free(ictx, "GPRS IMSI re-use");
}
}
strncpy(ctx->imsi, mi_string, sizeof(ctx->imsi));
@@ -776,7 +799,6 @@ err_inval:
static int gsm48_rx_gmm_det_req(struct sgsn_mm_ctx *ctx, struct msgb *msg)
{
struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(msg);
- struct sgsn_pdp_ctx *pdp, *pdp2;
uint8_t detach_type, power_off;
int rc;
@@ -789,26 +811,10 @@ static int gsm48_rx_gmm_det_req(struct sgsn_mm_ctx *ctx, struct msgb *msg)
msgb_tlli(msg), get_value_string(gprs_det_t_mo_strs, detach_type),
power_off ? "Power-off" : "");
- /* Mark MM state as deregistered */
- ctx->mm_state = GMM_DEREGISTERED;
-
- /* delete all existing PDP contexts for this MS */
- llist_for_each_entry_safe(pdp, pdp2, &ctx->pdp_list, list) {
- LOGMMCTXP(LOGL_NOTICE, ctx, "Dropping PDP context for NSAPI=%u "
- "due to GPRS DETACH REQUEST\n", pdp->nsapi);
- sgsn_delete_pdp_ctx(pdp);
- /* FIXME: the callback wants to transmit a DEACT PDP CTX ACK,
- * which is quite stupid for a MS that has just detached.. */
- }
-
/* force_stby = 0 */
rc = gsm48_tx_gmm_det_ack(ctx, 0);
- /* TLLI unassignment */
- gprs_llgmm_assign(ctx->llme, ctx->tlli, 0xffffffff,
- GPRS_ALGO_GEA0, NULL);
-
- sgsn_mm_ctx_free(ctx);
+ mm_ctx_cleanup_free(ctx, "GPRS DETACH REQUEST");
return rc;
}
diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c
index e68471313..75f8ae5f0 100644
--- a/openbsc/src/gprs/gprs_sgsn.c
+++ b/openbsc/src/gprs/gprs_sgsn.c
@@ -35,6 +35,7 @@
#include <openbsc/sgsn.h>
#include <openbsc/gsm_04_08_gprs.h>
#include <openbsc/gprs_gmm.h>
+#include "openbsc/gprs_llc.h"
extern struct sgsn_instance *sgsn;
@@ -247,11 +248,40 @@ struct sgsn_pdp_ctx *sgsn_pdp_ctx_alloc(struct sgsn_mm_ctx *mm,
}
#include <pdp.h>
-/* you probably want to call sgsn_delete_pdp_ctx() instead */
+/*
+ * This function will not trigger any GSM DEACT PDP ACK messages, so you
+ * probably want to call sgsn_delete_pdp_ctx() instead if the connection
+ * isn't detached already.
+ */
+void sgsn_pdp_ctx_terminate(struct sgsn_pdp_ctx *pdp)
+{
+ OSMO_ASSERT(pdp->mm != NULL);
+
+ /* There might still be pending callbacks in libgtp. So the parts of
+ * this object relevant to GTP need to remain intact in this case. */
+
+ LOGPDPCTXP(LOGL_INFO, pdp, "Forcing release of PDP context\n");
+
+ /* Force the deactivation of the SNDCP layer */
+ sndcp_sm_deactivate_ind(&pdp->mm->llme->lle[pdp->sapi], pdp->nsapi);
+
+ /* Detach from MM context */
+ llist_del(&pdp->list);
+ pdp->mm = NULL;
+
+ sgsn_delete_pdp_ctx(pdp);
+}
+
+/*
+ * Don't call this function directly unless you know what you are doing.
+ * In normal conditions use sgsn_delete_pdp_ctx and in unspecified or
+ * implementation dependent abnormal ones sgsn_pdp_ctx_terminate.
+ */
void sgsn_pdp_ctx_free(struct sgsn_pdp_ctx *pdp)
{
rate_ctr_group_free(pdp->ctrg);
- llist_del(&pdp->list);
+ if (pdp->mm)
+ llist_del(&pdp->list);
llist_del(&pdp->g_list);
/* _if_ we still have a library handle, at least set it to NULL
diff --git a/openbsc/src/gprs/sgsn_libgtp.c b/openbsc/src/gprs/sgsn_libgtp.c
index 44e94b3b8..b5285dc7f 100644
--- a/openbsc/src/gprs/sgsn_libgtp.c
+++ b/openbsc/src/gprs/sgsn_libgtp.c
@@ -263,6 +263,12 @@ static int create_pdp_conf(struct pdp_t *pdp, void *cbp, int cause)
LOGPDPCTXP(LOGL_INFO, pctx, "Received CREATE PDP CTX CONF, cause=%d(%s)\n",
cause, get_value_string(gtp_cause_strs, cause));
+ if (!pctx->mm) {
+ LOGP(DGPRS, LOGL_INFO,
+ "No MM context, aborting CREATE PDP CTX CONF\n");
+ return -EIO;
+ }
+
/* Check for cause value if it was really successful */
if (cause < 0) {
LOGP(DGPRS, LOGL_NOTICE, "Create PDP ctx req timed out\n");
@@ -314,16 +320,22 @@ reject:
static int delete_pdp_conf(struct pdp_t *pdp, void *cbp, int cause)
{
struct sgsn_pdp_ctx *pctx = cbp;
- int rc;
+ int rc = 0;
LOGPDPCTXP(LOGL_INFO, pctx, "Received DELETE PDP CTX CONF, cause=%d(%s)\n",
cause, get_value_string(gtp_cause_strs, cause));
- /* Deactivate the SNDCP layer */
- sndcp_sm_deactivate_ind(&pctx->mm->llme->lle[pctx->sapi], pctx->nsapi);
+ if (pctx->mm) {
+ /* Deactivate the SNDCP layer */
+ sndcp_sm_deactivate_ind(&pctx->mm->llme->lle[pctx->sapi], pctx->nsapi);
- /* Confirm deactivation of PDP context to MS */
- rc = gsm48_tx_gsm_deact_pdp_acc(pctx);
+ /* Confirm deactivation of PDP context to MS */
+ rc = gsm48_tx_gsm_deact_pdp_acc(pctx);
+ } else {
+ LOGPDPCTXP(LOGL_NOTICE, pctx,
+ "Not deactivating SNDCP layer since the MM context "
+ "is not available\n");
+ }
/* unlink the now non-existing library handle from the pdp
* context */
diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c
index 31c94f796..bfd5333d3 100644
--- a/openbsc/src/gprs/sgsn_vty.c
+++ b/openbsc/src/gprs/sgsn_vty.c
@@ -222,8 +222,9 @@ const struct value_string gprs_mm_st_strs[] = {
static void vty_dump_pdp(struct vty *vty, const char *pfx,
struct sgsn_pdp_ctx *pdp)
{
+ const char *imsi = pdp->mm ? pdp->mm->imsi : "(detaching)";
vty_out(vty, "%sPDP Context IMSI: %s, SAPI: %u, NSAPI: %u%s",
- pfx, pdp->mm->imsi, pdp->sapi, pdp->nsapi, VTY_NEWLINE);
+ pfx, imsi, pdp->sapi, pdp->nsapi, VTY_NEWLINE);
vty_out(vty, "%s APN: %s%s", pfx,
gprs_apn2str(pdp->lib->apn_use.v, pdp->lib->apn_use.l),
VTY_NEWLINE);