1445448 - Crash in sctp_handle_old_unordered_data

Status: RESOLVED WORKSFORME

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-e2eb74bc-0660-4e07-9aaa-0e0ee0180307.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll sctp_handle_old_unordered_data netwerk/sctp/src/netinet/sctp_indata.c:934
1 xul.dll sctp_deliver_reasm_check netwerk/sctp/src/netinet/sctp_indata.c:1096
2 xul.dll sctp_process_a_data_chunk netwerk/sctp/src/netinet/sctp_indata.c:2304
3 xul.dll sctp_process_data netwerk/sctp/src/netinet/sctp_indata.c:2771
4 xul.dll sctp_common_input_processing netwerk/sctp/src/netinet/sctp_input.c:6035
5 xul.dll usrsctp_conninput netwerk/sctp/src/user_socket.c:3490
6 xul.dll mozilla::DataChannelConnection::SctpDtlsInput netwerk/sctp/datachannel/DataChannel.cpp:821
7 xul.dll mozilla::TransportFlow::PacketReceived media/mtransport/transportflow.cpp:252
8 xul.dll sigslot::signal3<mozilla::TransportLayer*, unsigned char const*, unsigned __int64, sigslot::single_threaded>::operator media/mtransport/sigslot.h:2486
9 xul.dll mozilla::TransportLayerDtls::PacketReceived media/mtransport/transportlayerdtls.cpp:1026

=============================================================

this crash signature is starting to show up in 59 as a content crash with fairly low volume so far, perhaps introduced by bug 1297418.

Comment 1

[Daniel Veditz [:dveditz]]

Changing to UAF based on poison values showing up in the registry

drno: is there enough information here to make any progress?

Comment 2

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

dveditz: where do you see poison values?  I see 0xffffffff's (unknown) and wildptr (looks like random data, not a "real" pointer), but no poisons, even looking at Raw Dump register values.

Crashes all seem like they're 'control' getting trashed.  this are called under the input lock we added.  Some happen deeper in the function (on the FOREACH), which implies if it was control that was bad, that control *had* been good, and thus something in another thread somehow trashed it (or trashed our stack maybe -- unlikely).

Michael, Lenart - this appears to be a result of the landing of the updated SCTP library.  Very low frequency

Comment 3

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

change it back if I'm wrong about UAF, please.  (Could be a UAF, but I don't see poison, so I'm being cautious.)

Comment 4

[Daniel Veditz [:dveditz]]

I mostly don't see UAF so "wildptr" is fine. I do in the registers (r9 and rbx) in https://crash-stats.mozilla.com/report/index/bf8b4a7d-8973-4e6a-a02a-664390180315#tab-rawdump which was one of about 5 0xfffffffffff ones that I opened at random.

Comment 5

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

(In reply to Daniel Veditz [:dveditz] from comment #4)
> I mostly don't see UAF so "wildptr" is fine. I do in the registers (r9 and
> rbx) in
> https://crash-stats.mozilla.com/report/index/bf8b4a7d-8973-4e6a-a02a-
> 664390180315#tab-rawdump which was one of about 5 0xfffffffffff ones that I
> opened at random.

I opened about 1/2 dozen, saw no e5's in the regs (and there are none where it's the address we crash on), but that does look like it loaded some possible UAF data into local variables.  However, e5e5 can *also* be uninitialized data -- however, I think that's probably not the case here.  'control' in the dump is e5e5e5 (likely one of those registers).

'control' comes from one level up, via "control = TAILQ_FIRST(&strm->uno_inqueue);"

michael, lennart - thoughts?

Comment 6

[Michael Tüxen]

Do we have any information allowing to reproduce the problem?

Randall, which version of usrsctp is this?

Comment 7

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

0e076261b832121cf120ddc04aaff87ac3a34d30 from git on Tue Nov 28 15:20:51 EST 2017

No idea how to reproduce.  If I could reproduce it, 'rr' would solve the problem quickly in all likelihood

Comment 8

[Lennart Grahl [:lgrahl]]

(Would love to help but I really can't in this case.)

Comment 9

[Nils Ohlmeier [:drno]]

Appears to be Windows only.

Comment 10

[Nils Ohlmeier [:drno]]

Dan can I ask you to become the new owner since I have no cycles?

Comment 11

[Dan Minor [:dminor]]

Sure thing.

Comment 12

[Nils Ohlmeier [:drno]]

Looks like we don't really have much ideas left on how to get more information for this right now.

Comment 13

[Dan Minor [:dminor]]

Bouncing over to Nico as it sounds like he will be taking over data channel work.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Tiny volume, decreasing the priority

Comment 15

[Daniel Veditz [:dveditz]]

Not seeing this signature anymore

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

Since the bug is closed, the stalled keyword is now meaningless.
For more information, please visit auto_nag documentation.

Comment 17

[Daniel Veditz [:dveditz]]

jesup suspects it was fixed by bug 1400563 which landed last April (and is fixed in 68).