Regression: ofonod crashing on maguro on R62

apport information

apport information

apport information

So far none of us have been able to reproduce the crash on maguro/r62. I will work on analyzing the crash file next.

This *might* be related to the fact that an associated change to the lxc-android-config package was supposed to land in tandem with the new ofono version, but didn't.

A new plugin was added to ofono to handle SIM cards that don't properly report their mnc ( mobile-network-code ) length properly, however as the lxc-android-config update didn't land, the plugin is not being loaded by ofono.

The new version of lxc-android-config changes the ofonod exec line in /etc/init/ofono.override to:

exec ofonod -p ril,rilmodem,provision,mbpi,nettime,mnclength,smshistory

( it appends "mnclength" and "smshistory" )

Also, according to Dave, the crash only occurred on his phone when updated. He said he's re-flashed the phone with r62, and hasn't been able to get ofono to crash, although he has no GPRS. This latter problem with GPRS may be related to the fact that provisioning failed. Please add the contents of /var/lib/ofono/<IMSI>/gprs or run the script /usr/share/ofono/scripts/list-contexts and attach it's output. To force ofono to re-provision, the gprs file needs to have any contexts removed ( or you can just delete the file ), and then reboot. Only do this *after* modifying the upstart job as I suspect this whole series of problems may be related to mnclength, and the fact that the plugin wasn't loaded.

Here's the retracing of crash file:

Stacktrace:
 #0 parcel_r_int32 (p=p@entry=0xbefc4ab0) at gril/parcel.c:76
         ret = -1090762064
 #1 0x0001cdc0 in g_ril_reply_parse_sms_response (gril=0xc694e8, message=message@entry=0xc68780) at gril/grilreply.c:940
         rilp = {data = 0x0, offset = 0, capacity = 0, size = 0}
         error = <optimized out>
         mr = <optimized out>
         ack_pdu = <optimized out>
 #2 0x00022230 in ril_submit_sms_cb (message=0xc68780, user_data=0xc5fbc0) at drivers/rilmodem/sms.c:155
         cbd = 0xc5fbc0
         error = {type = OFONO_ERROR_TYPE_FAILURE, error = 0}
         cb = 0x7c44d <tx_finished>
         sd = 0xc694d8
         mr = <optimized out>
 #3 0x0001aba2 in handle_response (message=0xc68780, p=0xc65858) at gril/gril.c:348
         count = <optimized out>
         i = <optimized out>
         len = <optimized out>
         req = 0xc6c5a8
         found = 1
         id = <optimized out>
 #4 dispatch (message=0xc68780, p=<optimized out>) at gril/gril.c:496

So a couple of things collided to cause this crash.

First, as previously mentioned, there was an associated lxc-android-config package that was supposed to land in tandem with the ofono upload. I'm not sure why this didn't happen, we'll need to check with rsalveti.

I can't 100% blame the crash on the lack of the upstart changes in the lxc-android-config, but as the new mnclength plugin code was included, but not run, it has the potential to effect text messages.

The actual crash is in the code that handles the reply for a sent SMS message, and looking at the code, the reply was a FAILURE type, hence the error in frame #2 shows 'type = OFONO_ERROR_TYPE_FAILURE'.

The code crashes because the re-factored code in ril_submit_sms_cb() calls g_ril_reply_parse_sms_response() even though the reply has a non-SUCCESS error code *and* the g_ril_parse_sms_response() code fails to check a minimum length of the message ( which has length 0 ), before trying to read an int32 values from the ( empty ) parcel.

Originally, none of the rilmodem driver code ever attempted to parse an incoming message when an error was returned, however as we recently discovered that some replies did include event data even when an error is returned, not all of our code follows this convention. That said, I think the parse code should *only* be called when an error is returned if someone has verified that it's possible to for event_data to be returned anyways.

We also used to check a minimum length in the parse code, but it was pointed out that in some cases it's not possible to determine a safe minimum length if strings were involved. So... the parcel code was changed to be fail-safe and return 0 and mark itself as 'malformed' if not enough bytes are available to be read for a given data type. This code has been committed upstream in ofono/rilmodem, although as we haven't added logic to the parse routines to check the malformed flag, we haven't yet proposed a merge of this code into trunk. Merging this code would prevent the crash, however an alternative fix would be to just prevent the parsing of the SMS reply when an error is returned. As this is the simpler fix, I will create a MR for this change.

I will work with Alfonso on re-factoring the remaining parcel code to properly check the malformed flag, and hopefully we can land that next week.

This bug was fixed in the package ofono - 1.12+bzr6848-0ubuntu1

---------------
ofono (1.12+bzr6848-0ubuntu1) trusty; urgency=low

  [ Tony Espy ]
  * rilmodem/sms: Don't parse SMS error reply (LP: #1260388)
 -- Ricardo Salveti de Araujo <email address hidden> Fri, 13 Dec 2013 07:06:43 -0200