Closed
Bug 843595
Opened 10 years ago
Closed 10 years ago
WebRTC crash [@nr_ice_candidate_pair_destroy]
Categories
(Core :: WebRTC: Networking, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla22
Tracking | Status | |
---|---|---|
firefox21 | --- | disabled |
firefox22 | --- | fixed |
firefox-esr17 | --- | unaffected |
b2g18 | --- | unaffected |
People
(Reporter: posidron, Assigned: abr)
References
Details
(Keywords: crash, testcase, Whiteboard: [webrtc][blocking-webrtc+][qa-][adv-main22-])
Attachments
(3 files, 1 obsolete file)
This crash happened while fuzzing the SDP and seems to be a null ptr crash. ekr and I have decided to hide that bug first because of the free() function involved. The callstack contains also the output of NSPR_LOG. media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c:715 int nr_ice_candidate_pair_destroy(nr_ice_cand_pair **pairp) { nr_ice_cand_pair *pair; if(!pairp || !*pairp) return(0); pair=*pairp; *pairp=0; RFREE(pair->as_string); RFREE(pair->foundation); nr_ice_socket_deregister(pair->local->isock,pair->stun_client_handle); RFREE(pair->stun_client->params.ice_binding_request.username); <--
Reporter | ||
Comment 1•10 years ago
|
||
The problem seems to be a missing a=ice-ufrag attribute. Tested with m-i changeset: 122510:4e6834c859a8
Updated•10 years ago
|
Flags: in-testsuite?
Updated•10 years ago
|
Whiteboard: [webrtc][blocking-webrtc+][webrtc-uplift]
Updated•10 years ago
|
Assignee: nobody → adam
Priority: -- → P1
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #717906 -
Flags: review?(ekr)
Assignee | ||
Comment 3•10 years ago
|
||
I did a quick audit of all the object dtors to look for any similar issues. I found nothing glaring, but I added some paranoia checks in a couple of other places.
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #717906 -
Attachment is obsolete: true
Attachment #717906 -
Flags: review?(ekr)
Assignee | ||
Updated•10 years ago
|
Attachment #717906 -
Flags: review?(ekr)
Assignee | ||
Updated•10 years ago
|
Attachment #717916 -
Flags: review?(ekr)
Comment 5•10 years ago
|
||
Comment on attachment 717916 [details] [diff] [review] Null checks on pair dtor Review of attachment 717916 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/webrtc/signaling/test/signaling_unittests.cpp @@ +1990,5 @@ > + a2_.CreateAnswer(constraints, offer, OFFER_AV | ANSWER_AV); > + a2_.SetLocal(TestObserver::ANSWER, a2_.answer(), true); > + a1_.SetRemote(TestObserver::ANSWER, a2_.answer(), true); > + // We don't check anything in particular for success here -- simply not > + // crashing by now is enough to declare success. Don't you need a wait here to give the thread time to run?
Attachment #717916 -
Flags: review?(ekr) → review+
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Eric Rescorla (:ekr) from comment #5) > Comment on attachment 717916 [details] [diff] [review] > Null checks on pair dtor > > Review of attachment 717916 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: media/webrtc/signaling/test/signaling_unittests.cpp > @@ +1990,5 @@ > > + a2_.CreateAnswer(constraints, offer, OFFER_AV | ANSWER_AV); > > + a2_.SetLocal(TestObserver::ANSWER, a2_.answer(), true); > > + a1_.SetRemote(TestObserver::ANSWER, a2_.answer(), true); > > + // We don't check anything in particular for success here -- simply not > > + // crashing by now is enough to declare success. > > Don't you need a wait here to give the thread time to run? SetRemote() waits for a response (success or error) before it returns -- all the new "true" parameter suppresses is checking that the response was an error; it still waits for the response to happen.
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/77269eb211df
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/77269eb211df
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox22:
--- → fixed
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Updated•10 years ago
|
Whiteboard: [webrtc][blocking-webrtc+][webrtc-uplift] → [webrtc][blocking-webrtc+][webrtc-uplift][qa-]
Updated•10 years ago
|
Attachment #717906 -
Flags: review?(ekr)
Updated•10 years ago
|
status-b2g18:
--- → unaffected
status-firefox21:
--- → disabled
status-firefox-esr17:
--- → unaffected
Updated•10 years ago
|
Whiteboard: [webrtc][blocking-webrtc+][webrtc-uplift][qa-] → [webrtc][blocking-webrtc+][qa-]
Updated•10 years ago
|
Whiteboard: [webrtc][blocking-webrtc+][qa-] → [webrtc][blocking-webrtc+][qa-][adv-main22-]
Updated•10 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•