Comment 8 for bug 1260388

Revision history for this message
Tony Espy (awe) wrote :

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.