Hi Holger,

On Mon, Jul 29, 2013 at 10:53:18PM +0200, Holger Hans Peter Freyther wrote:
> I was doing a GPRS Attach and PDP Context Activation with one of the
> sysmocom GSM modules and my E71. Without the patches applied the E71
> fails to re-attach after a SGSN/PCU re-start with the same symptom
> (all our identity requests are sent with N(U) = 0). If you don't mind
> I will push these two changes to master.

Sure, go ahead. that's what I meant by 'be brave' :)

Thanks.

> I did this without a GGSN running and ran into the PDP Activcation
> Timeout... and a warning/backtrace. The below should fix it but I don't
> have time to verify that tonight.

This seems to be a bogus warning.  We do properly free the pdp library
handle, but we don't set pctx->lib to NULL in this path, see the below
suggested patch.

>From 4e9d0b8e3af3215f60bdbe75d419463828c7a32f Mon Sep 17 00:00:00 2001
From: Harald Welte <[email protected]>
Date: Tue, 30 Jul 2013 07:58:10 +0800
Subject: [PATCH] SGSN: set pctx->lib = NULL after calling
 pdp_freepdp(pctx->lib)

If we don't do this, we will generate a bogus warning in
gprs_sgsn:sgsn_pdp_ctx_free() when we release a pctx which still has the
lib pointer set to non-NULL.

At the same point, we introduce an OSMO_ASSERT() to verify that the
pctx->lib really equals the pdp context which libgtp reported to us.
---
 openbsc/src/gprs/sgsn_libgtp.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/openbsc/src/gprs/sgsn_libgtp.c b/openbsc/src/gprs/sgsn_libgtp.c
index f2eb35d..c244e2c 100644
--- a/openbsc/src/gprs/sgsn_libgtp.c
+++ b/openbsc/src/gprs/sgsn_libgtp.c
@@ -264,6 +264,10 @@ static int create_pdp_conf(struct pdp_t *pdp, void *cbp, 
int cause)
        DEBUGP(DGPRS, "Received CREATE PDP CTX CONF, cause=%d(%s)\n",
                cause, get_value_string(gtp_cause_strs, cause));
 
+       /* make sure that libgtp doesn't call us with inconsistent
+        * pointers */
+       OSMO_ASSERT(pctx->lib == pdp);
+
        /* Check for cause value if it was really successful */
        if (cause < 0) {
                LOGP(DGPRS, LOGL_NOTICE, "Create PDP ctx req timed out\n");
@@ -292,8 +296,13 @@ static int create_pdp_conf(struct pdp_t *pdp, void *cbp, 
int cause)
 
 reject:
        pctx->state = PDP_STATE_NONE;
-       if (pdp)
+       if (pdp) {
                pdp_freepdp(pdp);
+               /* unlink the now non-existing library handle from the
+                * pdp context */
+               pctx->lib = NULL;
+       }
+
        /* Send PDP CTX ACT REJ to MS */
        rc = gsm48_tx_gsm_act_pdp_rej(pctx->mm, pctx->ti, reject_cause,
                                        0, NULL);
-- 
1.8.3.2

-- 
- Harald Welte <[email protected]>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

Reply via email to