Closed Bug 825086 Opened 8 years ago Closed 8 years ago

Bad free in fsmdef_ev_create_answer()

Categories

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

x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla20
Tracking Status
firefox19 --- disabled
firefox20 --- fixed
firefox21 --- fixed
firefox-esr17 --- unaffected
b2g18 --- disabled

People

(Reporter: ekr, Assigned: abr)

Details

(Keywords: crash, sec-moderate, Whiteboard: [WebRTC] [blocking-webrtc+] [qa-])

Attachments

(1 file)

(gdb) bt
#0  0x00007fff85529499 in malloc_error_break ()
#1  0x00007fff85453183 in free ()
#2  0x00000001000fd865 in moz_free (ptr=0x121000000) at /Users/ekr/dev/mozilla-inbound/memory/mozalloc/mozalloc.cpp:48
#3  0x000000010462a07c in cc_free_msg_body_parts (msg_body=0x14b00fbf0) at /Users/ekr/dev/mozilla-inbound/media/webrtc/signaling/src/sipcc/core/gsm/ccapi.c:263
#4  0x00000001046405c2 in fsmdef_ev_createanswer (event=0x14b00fe28) at /Users/ekr/dev/mozilla-inbound/media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c:3087
#5  0x000000010466f7db in sm_process_event (tbl=0x1066b5698, event=0x14b00fe28) at /Users/ekr/dev/mozilla-inbound/media/webrtc/signaling/src/sipcc/core/gsm/sm.c:48
#6  0x00000001046300cf in fim_process_event (data=0x12ae50000, cac_passed=0 '\0') at /Users/ekr/dev/mozilla-inbound/media/webrtc/signaling/src/sipcc/core/gsm/fim.c:645
#7  0x0000000104652323 in gsm_process_msg (cmd=158, msg=0x12ae50000) at /Users/ekr/dev/mozilla-inbound/media/webrtc/signaling/src/sipcc/core/gsm/gsm.c:132
#8  0x0000000104652784 in GSMTask (arg=0x12b404700) at /Users/ekr/dev/mozilla-inbound/media/webrtc/signaling/src/sipcc/core/gsm/gsm.c:324
#9  0x00007fff85485fd6 in _pthread_start ()
#10 0x00007fff85485e89 in thread_start ()
(gdb)
Perhaps msg_body is borked here:

(gdb) p msg_body
$1 = {
  num_parts = 553648128, 
  content_type = cc_content_type_SDP, 
  parts = {{
      content_type = 553648128, 
      content_disposition = {
        disposition = cc_disposition_render, 
        required_handling = 128 ''
      }, 
      body_length = 1, 
      body = 0x121000000 "@", 
      content_id = 0x14b00fd00 "u-峯  㐀尀  ㄀∀ഀ    紀Ⰰ 笀ഀ      挀漀渀琀攀渀琀开琀礀瀀攀 㴀 ㄀㄀㠀㜀㐀㤀Ⰰ ഀ      挀漀渀琀攀渀琀开搀椀猀瀀漀猀椀琀椀漀渀 㴀 笀ഀ        搀椀猀瀀漀猀椀琀椀漀渀 㴀 挀挀开搀椀猀瀀漀猀椀琀椀漀渀开爀攀渀搀攀爀Ⰰ ഀ        爀攀焀甀椀爀攀搀开栀愀渀搀氀椀渀最 㴀   ✀尀 ✀ഀ      紀Ⰰ ഀ      戀漀搀礀开氀攀渀最琀栀 㴀 ㄀Ⰰ ഀ      戀漀搀礀 㴀  砀㄀   ㄀㔀挀昀㔀 ∀䠀쐀尀 ㈀ 崀쌀\017\037D", 
      content_id = 0x129a00000 "@"
    }, {
      content_type = 698351616, 
      content_disposition = {
        disposition = cc_disposition_render, 
        required_handling = 176 '➰ഀ      紀Ⰰ ഀ      戀漀搀礀开氀攀渀最琀栀 㴀 ㄀Ⰰ ഀ      戀漀搀礀 㴀  砀㄀   ㄀㔀挀昀㔀 ∀䠀쐀尀 ㈀ 崀쌀\017\037D", 
      content_id = 0x121000000 "@"
    }}
}
(gdb)
Assignee: nobody → adam
I suspect this is a result of one side offering a data channel and my having disabled it....
Keywords: crash
Priority: -- → P1
Whiteboard: [WebRTC] [blocking-webrtc+]
Okay, I'm going to wait on ekr's constraints patch to land before I dig into this.
This certainly looks like a block of uninitialized memory.

A hand check of the failure paths through gsmsdp_encode_sdp_and_update_version seems to indicate that any path to failure does not result in any memory being allocated. Consequently, the block that checks for failure of that function inside fsmdef_ev_createanswer should not be attempting to free the message body parts.

This over-aggressive freeing of body parts occurs four times. The impending patch fixes all four of them.
Attachment #697158 - Flags: review?(ekr)
Status: NEW → ASSIGNED
Comment on attachment 697158 [details] [diff] [review]
Removing errant free of body parts when encoding fails

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

lgtm
Attachment #697158 - Flags: review?(ekr) → review+
Attachment #697158 - Flags: checkin?(rjesup)
Attachment #697158 - Flags: checkin?(rjesup) → checkin+
https://hg.mozilla.org/mozilla-central/rev/d436c91b52c5
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [WebRTC] [blocking-webrtc+] → [WebRTC] [blocking-webrtc+] [qa-]
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.