1026037 - UAF of cc_call_info_t_

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Byron Campen [:bwc]]

Just saw an ASAN crash on my linux box:

    #0 0x786ada in strlib_is_string /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/src-common/string_lib.c:330
    #1 0x7869ec in strlib_copy /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/src-common/string_lib.c:95
    #2 0x5b2908 in calllogger_copy_call_log /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/call_logger.c:33
    #3 0x5d43c2 in getDeepCopyOfSessionData /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1078
    #4 0x5bae87 in CCAPI_Call_getCallInfo /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_call.c:30
    #5 0x5c7c58 in ccsnap_gen_callEvent /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_snapshot.c:438
    #6 0x5d1ce8 in ccappUpdateSessionData /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1633
    #7 0x5cb7c9 in ccp_handler /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:2026
    #8 0x5cae08 in CCApp_task /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccapp_task.c:184
    #9 0x43f293 in __asan::AsanThread::ThreadStart(unsigned long) (/home/bcampen/checkouts/mozilla-central/objdir-ff-asan/dist/cppunittests/signaling_unittests+0x43f293)
    #10 0x3620807f32 in start_thread (/lib64/libpthread.so.0+0x3620807f32)
    #11 0x36204f4eac in __clone (/lib64/libc.so.6+0x36204f4eac)
0x6040000a7f6c is located 28 bytes inside of 37-byte region [0x6040000a7f50,0x6040000a7f75)
freed by thread T0 here:
    #0 0x438c14 in __interceptor_free (/home/bcampen/checkouts/mozilla-central/objdir-ff-asan/dist/cppunittests/signaling_unittests+0x438c14)
    #1 0x5b2c79 in calllogger_free_call_log /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/call_logger.c:50
    #2 0x5bafcf in CCAPI_Call_releaseCallInfo /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccapi_call.c:60
    #3 0x597151 in CSF::CC_SIPCCCallInfo::~CC_SIPCCCallInfo() /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_ecc/../../../../../media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCallInfo.cpp:33
    #4 0x597103 in CSF::CC_SIPCCCallInfo::~CC_SIPCCCallInfo() /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_ecc/../../../../../media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCallInfo.cpp:32
    #5 0x529cd2 in CSF::CC_CallInfo::Release() /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_ecc/../../../../../media/webrtc/signaling/include/CC_CallInfo.h:32
    #6 0x576e1f in ~nsRefPtr /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_ecc/../../../../../xpcom/base/nsAutoPtr.h:870
    #7 0x576e1f in ~nsRefPtr /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_ecc/../../../../../xpcom/base/nsAutoPtr.h:868
    #8 0x576e1f in ~runnable_args_nm_3 /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_ecc/../../../../../media/mtransport/runnable_utils_generated.h:235
    #9 0x576e1f in ~runnable_args_nm_3 /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_ecc/../../../../../media/mtransport/runnable_utils_generated.h:235
    #10 0x576e1f in mozilla::runnable_args_nm_3<void (*)(nsAutoPtr<std::string>, ccapi_call_event_e, nsRefPtr<CSF::CC_CallInfo>), nsAutoPtr<std::string>, ccapi_call_event_e, nsRefPtr<CSF::CC_CallInfo> >::~runnable_args_nm_3() /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_ecc/../../../../../media/mtransport/runnable_utils_generated.h:235
    #11 0x1985124 in nsRunnable::Release() /home/bcampen/checkouts/latest-central/objdir-ff-asan/xpcom/glue/../../../xpcom/glue/nsThreadUtils.cpp:32
    #12 0x2b9130127040 in ~nsCOMPtr /home/bcampen/checkouts/mozilla-central/objdir-ff-asan/xpcom/threads/../../dist/include/nsCOMPtr.h:535
    #13 0x2b9130127040 in nsThread::ProcessNextEvent(bool, bool*) /home/bcampen/checkouts/mozilla-central/objdir-ff-asan/xpcom/threads/../../../xpcom/threads/nsThread.cpp:772
    #14 0x1986b0a in NS_ProcessNextEvent(nsIThread*, bool) /home/bcampen/checkouts/latest-central/objdir-ff-asan/xpcom/glue/../../../xpcom/glue/nsThreadUtils.cpp:263
    #15 0x486d67 in main /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signaling/test/../../../../../media/webrtc/signaling/test/signaling_unittests.cpp:4384
    #16 0x3620421d64 in __libc_start_main (/lib64/libc.so.6+0x3620421d64)
previously allocated by thread T14 here:
    #0 0x438cf4 in __interceptor_malloc (/home/bcampen/checkouts/mozilla-central/objdir-ff-asan/dist/cppunittests/signaling_unittests+0x438cf4)
    #1 0x2b913c3d63f2 in moz_xmalloc /home/bcampen/checkouts/latest-central/objdir-ff-asan/memory/mozalloc/../../../memory/mozalloc/mozalloc.cpp:52
    #2 0x786876 in strlib_malloc /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/src-common/string_lib.c:53
    #3 0x786b2e in strlib_update /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/src-common/string_lib.c:155
    #4 0x5b3943 in handlePlacedCall /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/call_logger.c:182
    #5 0x5d2458 in ccappUpdateSessionData /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:1721
    #6 0x5cb7c9 in ccp_handler /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccprovider.c:2026
    #7 0x5cae08 in CCApp_task /home/bcampen/checkouts/latest-central/objdir-ff-asan/media/webrtc/signalingtest/signaling_sipcc/../../../../../media/webrtc/signaling/src/sipcc/core/ccapp/ccapp_task.c:184
    #8 0x43f293 in __asan::AsanThread::ThreadStart(unsigned long) (/home/bcampen/checkouts/mozilla-central/objdir-ff-asan/dist/cppunittests/signaling_unittests+0x43f293)

   It seems that we have both main and sipcc's CCApp task touching the same data. main can destroy one of these when a CSF::CC_CallInfoPtr goes out of scope (this is essentially a nsRefPtr<CC_CallInfo>, which points at an instance of CC_SIPCCCallInfo, which contains a cc_callinfo_ref_t that it will release). CCApp task can also free these. Neither the refcount nor the container are threadsafe.

Comment 1

[Andrew McCreight [:mccr8]]

Byron, is this something you are still seeing?  Is there something we can do here or should it be closed as incomplete?  Thanks.

Comment 2

[Byron Campen [:bwc]]

I have some major work in progress that will solve this and other problems. ehugg might have cycles to put together a quick fix in the meantime, though, since this work is still a little ways out.

Comment 3

[Ethan Hugg [:ehugg]]

I'll take a look to see if there's a simple fix that can be done before your threading refactor lands.

Comment 4

[Byron Campen [:bwc]]

This should be fixed now that 991037 has landed.

Comment 5

[Daniel Veditz [:dveditz]]

Is there a subset of bug 991037 we could land to fix this on the ESR31 branch? Or does this not happen there for some reason?

Then again you didn't discover this until nightly was Firefox 33, maybe this is a regression and wouldn't happen on ESR31? I'd feel more confident in that if you can identify a regressing check-in or feature.

Comment 6

[Byron Campen [:bwc]]

I suspect that this was not a regression, and the backport would be gigantic. Ethan, do you have any cycles to take a crack at this?

Comment 7

[Ethan Hugg [:ehugg]]

So, this was found in the signaling unittests.  Do we have crash reports of this happening in Firefox?

I looked at this earlier and I don't think there's a simple fix to it and migrating the patch from 991037 would definitely be risky.

Comment 8

[Byron Campen [:bwc]]

I looked at crashstats for a bit, and didn't find anything with CCApp_task in the stack in the last year. And, I only had this happen to me once, in a test-case, where things like destruction of PCImpl aren't handled by the garbage collector. Might be a really hard bug to hit in the field.

Comment 9

[Al Billings [:abillings - ex-MoCo]]

Minusing this for ESR31. It doesn't seem like we're going to take this there.

Comment 10

[Kamil Jozwiak [:kjozwiak]]

I'm going to mark this as qe‑verify- due to the lack of poc/STR in this bug. If there's anything that QE and can do to verify this, please let us know and will take a another look!