807109 - Turn off SO_LINGER for usrsctp DataChannel sockets

Status: RESOLVED FIXED

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

Description

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

We do not want SCTP to try to send data through us after the underlying DTLS connections are torn down.

Comment 1

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

Attachment: Turn off SO_LINGER for SCTP sockets in DataChannels

Comment 2

[Eric Rescorla (:ekr)]

Comment on attachment 676776 [details] [diff] [review]
Turn off SO_LINGER for SCTP sockets in DataChannels

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

::: netwerk/sctp/datachannel/DataChannel.cpp
@@ +217,5 @@
> +  struct linger l;
> +  l.l_onoff = 1;
> +  l.l_linger = 0;
> +  if (usrsctp_setsockopt(mMasterSocket, SOL_SOCKET, SO_LINGER,
> +                         (const void *)&l, (socklen_t)sizeof(struct linger)) < 0) {

I usually use C++-style casts in C++ code. I don't know what Moz convention is.

Is socklen_t always the same size as size_t?

Comment 3

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

(In reply to Eric Rescorla from comment #2)
> ::: netwerk/sctp/datachannel/DataChannel.cpp
> @@ +217,5 @@
> > +  struct linger l;
> > +  l.l_onoff = 1;
> > +  l.l_linger = 0;
> > +  if (usrsctp_setsockopt(mMasterSocket, SOL_SOCKET, SO_LINGER,
> > +                         (const void *)&l, (socklen_t)sizeof(struct linger)) < 0) {
> 
> I usually use C++-style casts in C++ code. I don't know what Moz convention
> is.

Generally C++ style, though this is calling a C lib.  In any case, this is the same style used for all the other usrsctp_setsockopt() calls already in DataChannel.cpp.

> Is socklen_t always the same size as size_t?

You'd think so, but it's a distinct type and that's what the API wants.  (In fact it was broken on x64 originally because of how they defined it if it wasn't already defined; I fixed that and fed it back to them.)

Thanks

Comment 4

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/5eb2dc37b784

If we don't take the Bug 807103 fix for FF18, there's no way for this to be provoked.  If we provide some way for DataChannelConnections to actually be deleted in 18, we will need to take this fix.

Comment 5

[Eric Rescorla (:ekr)]

(In reply to Randell Jesup [:jesup] from comment #3)
> (In reply to Eric Rescorla from comment #2)
> > ::: netwerk/sctp/datachannel/DataChannel.cpp
> > @@ +217,5 @@
> > > +  struct linger l;
> > > +  l.l_onoff = 1;
> > > +  l.l_linger = 0;
> > > +  if (usrsctp_setsockopt(mMasterSocket, SOL_SOCKET, SO_LINGER,
> > > +                         (const void *)&l, (socklen_t)sizeof(struct linger)) < 0) {
> > 
> > I usually use C++-style casts in C++ code. I don't know what Moz convention
> > is.
> 
> Generally C++ style, though this is calling a C lib.  In any case, this is
> the same style used for all the other usrsctp_setsockopt() calls already in
> DataChannel.cpp.
> 
> > Is socklen_t always the same size as size_t?
> 
> You'd think so, but it's a distinct type and that's what the API wants.  (In
> fact it was broken on x64 originally because of how they defined it if it
> wasn't already defined; I fixed that and fed it back to them.)

What I'm concerned about is if it's of a different size. In that case this cast
is potentially dangerous.

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5eb2dc37b784