810626 - WebRTC use-after-free crash [@ mozilla::TransportLayer::SetState]

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

alloc: media/webrtc/signaling/src/mediapipeline/MediaPipeline.h:58
    if (!muxed_) {
      rtcp_transport_->SignalStateChange.connect(this,
                                                 &MediaPipeline::StateChange);


free: media/webrtc/signaling/src/mediapipeline/MediaPipeline.h:178

  virtual ~MediaPipelineTransmit() {
    if (stream_ && listener_){
      stream_->GetStream()->RemoveListener(listener_);


re-use: media/mtransport/transportlayer.cpp:48

    SignalStateChange(this, state);


Tested with m-c changeset: 112919:ea5c4c1b0edf

Comment 1

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

ekr: have you looked at this?  It's a sec-critical (though mitigated by only being accessed behind a pref).  (Would that be status-firefoxNN: disabled?)

Comment 2

[Eric Rescorla (:ekr)]

Yes. I suspect this will be fixed by the big lock patch. I agree it's not good.

Comment 3

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

Eric: please link it as a dependency of the big lock patch.  Thanks

Comment 4

[Jason Smith [:jsmith]]

Doesn't look crash test worthy at the moment, unless there's a reproducible test case from HTML that exists that I'm not seeing.

Comment 5

[Jason Smith [:jsmith]]

Re-test when the peer connection lock patch lands.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Jason, has the lock patch landed?

Comment 7

[Jason Smith [:jsmith]]

Redirecting question to ekr

Comment 8

[Eric Rescorla (:ekr)]

No.

Comment 9

[Ethan Hugg [:ehugg]]

That patch is here:

https://bugzilla.mozilla.org/show_bug.cgi?id=792175

Comment 10

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

Excerpt from the attached callstack for better querying on Bugzilla:

    #0 0x1089cbfb6 in std::_List_const_iterator<sigslot::_connection_base2<mozilla::TransportFlow*, mozilla::TransportLayer::State, sigslot::single_threaded>*>::operator++ stl_list.h:224
    #1 0x1089cb5f1 in sigslot::signal2<mozilla::TransportFlow*, mozilla::TransportLayer::State, sigslot::single_threaded>::operator sigslot.h:2409
    #2 0x1089d11e7 in sigslot::signal2<mozilla::TransportLayer*, mozilla::TransportLayer::State, sigslot::single_threaded>::operator sigslot.h:2411
    #3 0x1089d0de8 in mozilla::TransportLayer::SetState transportlayer.cpp:48
    #4 0x1089daaf9 in mozilla::TransportLayerDtls::Handshake transportlayerdtls.cpp:671
    #5 0x1089d96bb in mozilla::TransportLayerDtls::PacketReceived transportlayerdtls.cpp:692

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

Lock patch landed. We need to see if this reproduces on trunk.

Comment 12

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

Cristoph - any sign this is still occurring?

Comment 13

[Christoph Diehl [:posidron]]

Nope. I am not seeing it anymore.

Comment 14

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

Closing per above; please re-open if it recurs in ASAN testing. Thanks!

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Looks like this patch did land on 20 in bug 792175 so marking the status flag accordingly.