diff options
author | Jacob Erlbeck <jerlbeck@sysmocom.de> | 2014-10-13 10:32:00 +0200 |
---|---|---|
committer | Holger Hans Peter Freyther <holger@moiji-mobile.com> | 2014-10-27 10:25:13 +0100 |
commit | 99985b5ea8e2d69d1e63a9423fbe40b872b0c0f5 (patch) | |
tree | 06257bc6fdf1bfbc1f260fdf2e54e448746eba52 /openbsc/src | |
parent | ae20b4b31b679d7ea057f4db8427b99293068ec5 (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
Diffstat (limited to 'openbsc/src')
-rw-r--r-- | openbsc/src/gprs/gprs_gmm.c | 54 | ||||
-rw-r--r-- | openbsc/src/gprs/gprs_sgsn.c | 34 | ||||
-rw-r--r-- | openbsc/src/gprs/sgsn_libgtp.c | 22 | ||||
-rw-r--r-- | openbsc/src/gprs/sgsn_vty.c | 3 |
4 files changed, 81 insertions, 32 deletions
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); |