Bug 1021335: Fix DataChannel leak of recv buffers from SCTP r=tuexen
authorRandell Jesup <rjesup@jesup.org>
Sun, 08 Jun 2014 09:53:13 -0400
changeset 206721 1159503d1f38f745a105b7484f70104021d193a6
parent 206720 229dc47b5059d1feb9a32af179e0141616a482a7
child 206722 460666c9b930280541d0044a08a85d572e8184ab
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstuexen
bugs1021335
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1021335: Fix DataChannel leak of recv buffers from SCTP r=tuexen
netwerk/sctp/datachannel/DataChannel.cpp
--- a/netwerk/sctp/datachannel/DataChannel.cpp
+++ b/netwerk/sctp/datachannel/DataChannel.cpp
@@ -1313,25 +1313,26 @@ DataChannelConnection::HandleDataMessage
     // before we're told about the external negotiation.  We need to buffer
     // data until either a) Open comes in, if the ordering get messed up,
     // or b) the app tells us this channel was externally negotiated.  When
     // these occur, we deliver the data.
 
     // Since this is rare and non-performance, keep a single list of queued
     // data messages to deliver once the channel opens.
     LOG(("Queuing data for stream %u, length %u", stream, length));
+    // Copies data
     mQueuedData.AppendElement(new QueuedDataMessage(stream, ppid, data, length));
     return;
   }
 
   // XXX should this be a simple if, no warnings/debugbreaks?
   NS_ENSURE_TRUE_VOID(channel->mState != CLOSED);
 
   {
-    nsAutoCString recvData(buffer, length);
+    nsAutoCString recvData(buffer, length); // copies (<64) or allocates
     bool is_binary = true;
 
     if (ppid == DATA_CHANNEL_PPID_DOMSTRING ||
         ppid == DATA_CHANNEL_PPID_DOMSTRING_LAST) {
       is_binary = false;
     }
     if (is_binary != channel->mIsRecvBinary && !channel->mRecvBuffer.IsEmpty()) {
       NS_WARNING("DataChannel message aborted by fragment type change!");
@@ -1928,16 +1929,21 @@ DataChannelConnection::ReceiveCallback(s
   } else {
     MutexAutoLock lock(mLock);
     if (flags & MSG_NOTIFICATION) {
       HandleNotification(static_cast<union sctp_notification *>(data), datalen);
     } else {
       HandleMessage(data, datalen, ntohl(rcv.rcv_ppid), rcv.rcv_sid);
     }
   }
+  // sctp allocates 'data' with malloc(), and expects the receiver to free
+  // it (presumably with free).
+  // XXX future optimization: try to deliver messages without an internal
+  // alloc/copy, and if so delay the free until later.
+  free(data);
   // usrsctp defines the callback as returning an int, but doesn't use it
   return 1;
 }
 
 already_AddRefed<DataChannel>
 DataChannelConnection::Open(const nsACString& label, const nsACString& protocol,
                             Type type, bool inOrder,
                             uint32_t prValue, DataChannelListener *aListener,