From: Richard Röjfors <rich...@puffinpack.se>

There was an issue while running LTE and the connection
manager tried to activate the context with CID 1 while
it got automatically activated at the same time with
CID 4.

When the automatic activation happened ofono_gprs_cid_activated
got called which tried to assign the context, but that failed
since the driver context was considered in use
(by the activation call).
Eventhough it failed, the context was modified,
cid was set to 0 (making cid 1 leak).
Then release_context got called which clear pointers
assigned to the context.

A bit later the activation callback got called, in my case
activation failed. Due to the failure it tries to clean up
by calling context_settings_free, but unfortunately the pointers
where reset above causing ofono to segfault du to null pointer
derefs.

Instead we make sure assign_context does not touch the context
unless it succeeds. Then there is no need to call release_context
if assign fails.
That ensures the context being intact when the activation callback
gets called.

03:23:21 ofonod[545]: Aux: < \r\n+CGEV: ME PDN ACT 4\r\n\r\n+CTZE: 
+04,0,"19/12/10,04:25:03"\r\n
03:23:21 ofonod[545]: drivers/ubloxmodem/network-registration.c:ctze_notify() 
tz +04 dst 0 time 19/12/10,04:25:03
03:23:21 ofonod[545]: src/network.c:ofono_netreg_time_notify() net time 
2019-12-10 04:25:03 utcoff 3600 dst 0
03:23:22 ofonod[545]: Aux: > AT+CGDCONT?\r
03:23:22 ofonod[545]: 
drivers/ubloxmodem/gprs-context.c:ublox_gprs_activate_primary() cid 1

Connection manager requests activation, will mark the context in use and assign
it cid 1.

03:23:22 ofonod[545]: Aux: < \r\n+CGDCONT: 
1,"IP","m2m.tele2.com","",0,0,0,0,0,0\r\n
03:23:22 ofonod[545]: Aux: < +CGDCONT: 
4,"IP","m2m.tele2.com.mnc003.mcc248.gprs","100.69.174.133",0,0,0,0,0,0\r\n
03:23:22 ofonod[545]: Aux: < \r\nOK\r\n
03:23:22 ofonod[545]: drivers/atmodem/gprs.c:at_cgdcont_read_cb() ok 1
03:23:22 ofonod[545]: src/gprs.c:ofono_gprs_cid_activated() cid 4
03:23:22 ofonod[545]: Can't assign context to driver for APN.

Since its marked in use above, we fail to assign it cid 4. When that fails
the cid is cleared an all context pointers are set to NULL.

03:23:22 ofonod[545]: Aux: > AT+CGDCONT=1,"IP","m2m.tele2.com"\r
03:23:22 ofonod[545]: Aux: < \r\nOK\r\n
03:23:22 ofonod[545]: drivers/ubloxmodem/gprs-context.c:cgdcont_cb() ok 1
03:23:22 ofonod[545]: Aux: > AT+CGACT=1,1\r
03:23:22 ofonod[545]: Aux: < \r\n+CME ERROR: 100\r\n
03:23:22 ofonod[545]: drivers/ubloxmodem/gprs-context.c:cgact_enable_cb() ok 0
03:23:22 ofonod[545]: src/gprs.c:pri_activate_callback() 0x853480
03:23:22 ofonod[545]: src/gprs.c:pri_activate_callback() Activating context 
failed with error: Unknown error

Activation callback, and it failed. Will try to clean up, but the pointers are
NULL'ed...

Dec 10 03:23:22 ofonod[545]: Aborting (signal 11) [/usr/sbin/ofonod]
---
 src/gprs.c | 64 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index 2d3a99e6..223c777f 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -223,23 +223,11 @@ static gboolean gprs_context_string_to_type(const char 
*str,
        return FALSE;
 }
 
-static gboolean assign_context(struct pri_context *ctx, unsigned int use_cid)
+static struct ofono_gprs_context *
+find_avail_gprs_context(struct pri_context *ctx)
 {
-       struct l_uintset *used_cids = ctx->gprs->used_cids;
        GSList *l;
 
-       if (used_cids == NULL)
-               return FALSE;
-
-       if (!use_cid)
-               use_cid = l_uintset_find_unused_min(used_cids);
-
-       if (use_cid > l_uintset_get_max(used_cids))
-               return FALSE;
-
-       l_uintset_put(used_cids, use_cid);
-       ctx->context.cid = use_cid;
-
        for (l = ctx->gprs->context_drivers; l; l = l->next) {
                struct ofono_gprs_context *gc = l->data;
 
@@ -257,24 +245,45 @@ static gboolean assign_context(struct pri_context *ctx, 
unsigned int use_cid)
                                gc->type != ctx->type)
                        continue;
 
-               ctx->context_driver = gc;
-               ctx->context_driver->inuse = TRUE;
+               return gc;
+       }
 
-               if (ctx->context.proto == OFONO_GPRS_PROTO_IPV4V6 ||
-                               ctx->context.proto == OFONO_GPRS_PROTO_IP)
-                       gc->settings->ipv4 = g_new0(struct ipv4_settings, 1);
+       return NULL;
+}
 
-               if (ctx->context.proto == OFONO_GPRS_PROTO_IPV4V6 ||
-                               ctx->context.proto == OFONO_GPRS_PROTO_IPV6)
-                       gc->settings->ipv6 = g_new0(struct ipv6_settings, 1);
+static gboolean assign_context(struct pri_context *ctx, unsigned int use_cid)
+{
+       struct l_uintset *used_cids = ctx->gprs->used_cids;
+       struct ofono_gprs_context *gc;
 
-               return TRUE;
-       }
+       if (used_cids == NULL)
+               return FALSE;
 
-       l_uintset_take(used_cids, ctx->context.cid);
-       ctx->context.cid = 0;
+       if (!use_cid)
+               use_cid = l_uintset_find_unused_min(used_cids);
 
-       return FALSE;
+       if (use_cid > l_uintset_get_max(used_cids))
+               return FALSE;
+
+       gc = find_avail_gprs_context(ctx);
+       if (gc == NULL)
+               return FALSE;
+
+       l_uintset_put(used_cids, use_cid);
+       ctx->context.cid = use_cid;
+
+       ctx->context_driver = gc;
+       ctx->context_driver->inuse = TRUE;
+
+       if (ctx->context.proto == OFONO_GPRS_PROTO_IPV4V6 ||
+                       ctx->context.proto == OFONO_GPRS_PROTO_IP)
+               gc->settings->ipv4 = g_new0(struct ipv4_settings, 1);
+
+       if (ctx->context.proto == OFONO_GPRS_PROTO_IPV4V6 ||
+                       ctx->context.proto == OFONO_GPRS_PROTO_IPV6)
+               gc->settings->ipv6 = g_new0(struct ipv6_settings, 1);
+
+       return TRUE;
 }
 
 static void release_context(struct pri_context *ctx)
@@ -2032,7 +2041,6 @@ void ofono_gprs_cid_activated(struct ofono_gprs  *gprs, 
unsigned int cid,
 
        if (assign_context(pri_ctx, cid) == FALSE) {
                ofono_warn("Can't assign context to driver for APN.");
-               release_context(pri_ctx);
                return;
        }
 
-- 
2.20.1
_______________________________________________
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

Reply via email to