793863 - Signaling code: crash in fsmdef_release

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[Ethan Hugg [:ehugg]]

This crash happened with Asan more than once, but is not easily repeatable:

    ==43752== ERROR: AddressSanitizer crashed on unknown address 0x000000000138 (pc 0x00011e0bb1b4 sp 0x000153bf8700 bp 0x000153bf9270 T37)
    AddressSanitizer can not provide additional info. ABORTING
        #0 0x11e0bb1b3 in fsmdef_release fsmdef.c:1812
        #1 0x11e0ec4f3 in fsmdef_ev_addstream fsmdef.c:3445
        #2 0x11e2599fb in sm_process_event sm.c:83
        #3 0x11e0625e3 in fim_process_event fim.c:671
        #4 0x11e16e730 in gsm_process_msg gsm.c:167
        #5 0x11e16fcfa in GSMTask gsm.c:359
        #6 0x10003626a in __asan::AsanThread::ThreadStart() (in firefox) + 42
        #7 0x7fff8b9f7181 in thread_start (in libsystem_c.dylib) + 13
    Stats: 3533M malloced (3054M for red zones) by 4888867 calls
    Stats: 928M realloced by 120042 calls
    Stats: 3312M freed by 4643955 calls
    Stats: 3179M really freed by 4380448 calls
    Stats: 1493M (382210 full pages) mmaped in 352 calls
      mmaps   by size class: 8:376809; 9:49146; 10:245700; 11:10235; 12:5120; 13:40448; 14:1280; 15:896; 16:512; 17:224; 18:240; 19:152; 20:172; 21:128; 22:3; 25:3;
      mallocs by size class: 8:1147717; 9:142267; 10:3481376; 11:21238; 12:26770; 13:47755; 14:9059; 15:5890; 16:2875; 17:1474; 18:1379; 19:350; 20:434; 21:270; 22:10; 25:3;
      frees   by size class: 8:981607; 9:120848; 10:3472026; 11:17294; 12:23241; 13:8387; 14:8471; 15:5639; 16:2644; 17:1447; 18:1293; 19:346; 20:431; 21:268; 22:10; 25:3;
      rfrees  by size class: 8:927622; 9:113512; 10:3273106; 11:16416; 12:22110; 13:7983; 14:8065; 15:5411; 16:2536; 17:1357; 18:1276; 19:345; 20:430; 21:267; 22:9; 25:3;
    Stats: malloc large: 4025 small slow: 38383

Comment 1

[Ethan Hugg [:ehugg]]

Attachment: Signaling code - check for fcb already released

Comment 2

[Ethan Hugg [:ehugg]]

Comment on attachment 664262 [details] [diff] [review]
Signaling code - check for fcb already released


I found several instances of this check-for-null and double-return.  I changed them to return SM_RC_CLEANUP which according to the comments should signal we're done with this data.  I assume this is happening because it's already run through release, but I can't prove that.

Also protected fsmdef_release from any other call with a dcb of null, and removed an instance of fname in favor of __FUNCTION__

Comment 3

[Eric Rescorla (:ekr)]

Comment on attachment 664262 [details] [diff] [review]
Signaling code - check for fcb already released

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

lgtm with one nit

::: media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c
@@ +2913,5 @@
>      }    
>      
>      if (dcb == NULL) {
>      	FSM_DEBUG_SM(DEB_F_PREFIX"dcb is NULL.\n", DEB_F_PREFIX_ARGS(FSM, __FUNCTION__));
> +      return SM_RC_CLEANUP;

Funny indent.

Comment 4

[Ethan Hugg [:ehugg]]

Attachment: Signaling code - check for fcb already released

Comment 5

[Ethan Hugg [:ehugg]]

Comment on attachment 664276 [details] [diff] [review]
Signaling code - check for fcb already released


Pushed to Alder - http://hg.mozilla.org/projects/alder/rev/9145a0eed02d

Leaving the bug open until confirmation from cdiehl that we didn't just move the problem elsewhere.

Comment 6

[Christoph Diehl [:posidron]]

I am getting now the following crash:

==83659== ERROR: AddressSanitizer crashed on unknown address 0x000000000000 (pc 0x00011e4005d2 sp 0x000161f32800 bp 0x000161f32a50 T36)
AddressSanitizer can not provide additional info. ABORTING
    #0 0x11e4005d1 in fsm_init_fcb fsm.c:147
    #1 0x11e44c192 in fsmdef_free_cb fsmdef.c:1227
    #2 0x11e3fc7ee in fim_free_call_chn fim.c:162
    #3 0x11e3f88dc in fim_process_event fim.c:718
    #4 0x11e5038e0 in gsm_process_msg gsm.c:167
    #5 0x11e504eaa in GSMTask gsm.c:359
    #6 0x10003622a in __asan::AsanThread::ThreadStart() (in firefox) + 42
    #7 0x7fff8b9f7181 in thread_start (in libsystem_c.dylib) + 13
Stats: 17227M malloced (12751M for red zones) by 15490798 calls
Stats: 9135M realloced by 438706 calls
Stats: 16015M freed by 14952769 calls
Stats: 15847M really freed by 14877742 calls
Stats: 3747M (959279 full pages) mmaped in 915 calls

Comment 7

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

Fixed in bug 805701