805701 - WebRTC crash [@fsm_init_fcb]

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

This might be caused by the same issue as in https://bugzilla.mozilla.org/show_bug.cgi?id=805691 
I can not get a workable testcase for both bugs but they occur on a regular basis during SDP fuzzing. Will try to provide more information.

media/webrtc/signaling/src/sipcc/core/gsm/fsm.c

void
fsm_init_fcb (fsm_fcb_t *fcb, callid_t call_id, fsmdef_dcb_t *dcb,
              fsm_types_t type)
{
    fcb->call_id = call_id;
[...]

media/webrtc/signaling/src/sipcc/core/gsm/fim.c
[...]
    for (icb = call_chn; icb != NULL; icb = icb->next_icb) {
        if (icb->scb->free_cb != NULL) {
            icb->scb->free_cb(icb, icb->call_id);
        }
        icb->call_id = CC_NO_CALL_ID;
        icb->cb = NULL;
    }
[...]

Tested on m-central with changeset: 111360:58c8080a1a7c

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

While that callstack looks impossible, there's very likely a fsmdef.c:1194 inbetween frames 0 and 1.

That's the only free_cb pointer value that has a tail-call to fsm_init_cb *and* doesn't check fsm for NULL.

Why it would be NULL is another question, but certainly all the other init callers generally check against NULL, so probably best to just add the null check

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: protect against NULL fcb pointer

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This crash can be seen on OS X with my testcase from bug 798323 (attachment 671342 [details]).

https://crash-stats.mozilla.com/report/index/fcba0483-1c28-4d90-81c5-709ef2121027

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Just one mre note, you will have to let run Nightly for a while with the testcase open. It should crash after about 2 minutes.

Comment 5

[Ethan Hugg [:ehugg]]

Comment on attachment 675810 [details] [diff] [review]
protect against NULL fcb pointer

Review of attachment 675810 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with syntax error fixed.  Although this is probably a symptom of a larger problem.  We should re-test after lock patch.

::: media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c
@@ +1192,5 @@
>              fsmdef_init_dcb(dcb, CC_NO_CALL_ID, FSMDEF_CALL_TYPE_NONE,
>                              NULL, LSM_NO_LINE, NULL);
> +            /* fsmdef_init_dcb(...,NULL) will always set the fcb ptr to NULL,
> +               so if fsmdef_free_cb were called on that we'd have fcb==NULL here */
> +            if (fcb != NULL) {

I believe we're missing a closing brace here.  Syntax error.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/19dc5eccbc06

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/19dc5eccbc06

Comment 8

[Jason Smith [:jsmith]]

Not crash test worthy since another bug already covers the crashtest likely, although the other bug referenced by whimboo may be crashtest worthy that will cover this bug inevitably.