sysmocom
/
ofono
mirror of git://git.sysmocom.de/ofono
11
0
Fork 0
Code Issues Releases Activity

gprs: Don't modify the context if assign fails 

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]
This commit is contained in:
Richard Röjfors 2019-12-10 13:58:16 +01:00 committed by Denis Kenzior
parent d7d49eb1d5
commit 3b7d8e003e
1 changed files with 38 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
src/gprs.c
View File

@ -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;
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;
return gc;
}
l_uintset_take(used_cids, ctx->context.cid);
ctx->context.cid = 0;
return NULL;
}
return FALSE;
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;
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;
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;
}