Possible Memory corruption in Rtp/RtspClient

Topics: Question
May 27, 2016 at 2:42 PM
Revision 112081.
I received bad RtpPacket that contains "RTSP/1.0 200 OK".
I assume RtspClient.m_Buffer, RtpClient.m_Buffer, and TransportContext.ContextMemory are same instance and have no lock when socket receiving.
Is it correct?
May 27, 2016 at 3:12 PM
Edited May 27, 2016 at 3:12 PM
Hello again... and thanks for your interest in the project.

Can you provide a dump showing what you experience? Is this from the RtspClient or RtspServer?

Yes, RtspClient.m_Buffer is shared with RtpClient buffer as well as the ContextMemory but there is no locking required because the buffers are only accessed after socket send or receive and poll, furthermore your are incorrect about NO LOCKS, there is method level synchronization via the Synchronized attribute for SendRtspMessage and for SendRtpPacket /SendRtcpPacket...
Marked as answer by juliusfriedman on 5/27/2016 at 7:12 AM
May 28, 2016 at 4:52 AM
Thanks for reply.
Is this from the RtspClient. Because it is very rare, I cannot prepare the dump immediately.

I think
RtpClient.ReceiveData (from SendReceieve thread)
RtspClient.SendRtspMessage (from MonitorProtocol thread or Main thread)
can run simultaneously.

..and SendRtspMessage really have Synchronized attribute?, It's effective in different objects(RtspClient/RtpClient)?
May 28, 2016 at 2:39 PM
Edited May 29, 2016 at 6:12 AM
You think and you are right. I want to eventually replace the Timers with a single Thread in the RtspClient and eventually multiple threads per TransportContext for the RtpClient.

That is more or less an extra performance thing and in that time the buffer will still be shared but the offsets and lengths in the buffer will be dedicated which will ensure no locking is required.

Sockets are Full Duplex which means I can send and receive at the same time especially with Send and Receive buffer sizes @ 0 and no coalescing under TCP.

When the Timers run or a message is sent InUse is checked along with Reset of the ManualResetEvent which only is used to ensure that if Blocking sockets were utilized that the Blocking timeout would happen less and when not that there would be less chance of a double poll when a single socket was used e.g. in SharesSocket.

Synchronized is just as effective as lock(somethingElse) when your only using it for one thing, e.g. sending Rtp or Rtcp or Rtsp, when used in a static it locks the type.

Finally Rtsp was designed with an errata for the $ framing because it is both a valid UTF8 character and listed in the SAFE grammar in the RFC (which is dumb at best because even HTTP doesn't allow it so they thought they were cute and used a character which appears not only in legit data from Rtsp but which also signals their start of frame)

This means when in some cases you have $ at the offset 0, the RtpClient will receive this data, in something LIKE ffmpeg it's skipped, where as my lib actually allows for it to be chunked to the upper layer to complete the RtspMessage.

Thus the RtpClient may say it has an packet but there will be no TransportContext associated unless the packet was specially crafted in a way that it does, which is also dealt with by my other packet verification functions.

I hope this answers your questions and eliminates your thoughts on memory corruption, further more such corruption would be met with exception by the CLR if indeed was the case.

Worse, they [IETF] move(d) to make 'independent' framing e.g. RFC4571 standard in RTSP 2.0, but they don't say how one you would use both interleaved and independent on the same connection, they don't say how proxies are to handle ssrc or payload overlaps (e.g. for proxies) without the channel from the framing and finally they don't even provide a decent list of checks or logic to perform when any of this happens, they just say to 'skip' the data. (not even provide it to the upper layer) which is totally wrong and causes legit messages from RTSP etc to be missed or 'corrupted'

I believe you simply mean that you don't understand how data is passed around in the library.

Futhermore I think you mean that you don't understand how and when to lock.

See Also

See the UnitTests which demonstrate this then try the same with another library :)
Marked as answer by juliusfriedman on 5/28/2016 at 6:39 AM
May 31, 2016 at 12:10 AM
Edited May 31, 2016 at 12:12 AM
Screenshots are here. (Break at OnRtpPacketReceieved(). Locale of VS is Japanese)
I think 0x52 0x54 0x53 0x50 .... is response of RTSP GET_PARAMETER.
I believe you simply mean that you don't understand how data is passed around in the library.
I will check what it is caused.

May 31, 2016 at 9:59 PM
Edited May 31, 2016 at 10:14 PM
Well, it seems you know how to inspect the memory related to the object which is good.

Now, It would help if you could provide a small capture which shows if the library incorrectly picked this up as a RtspMessage (which is possible) or if it just provided an event which was handled elsewhere to just itself know that data was received possibly out of band.

Finally, even with this issue I would imagine that the library still can communicate with the source but you just take a little longer to actually get the event for the message. If so this will improved later on this week.

Also, If you have any SessionDescription or RtspMessage's which correctly exhibit the use of an Encoding besides UTF8 I would be happy to include such tests in the library as well as support for the 'charset' and 'sdplang' lines, I just need a good example :)
Marked as answer by juliusfriedman on 5/31/2016 at 1:59 PM
Jul 8, 2016 at 2:30 AM
This issue should be fixed.

If you have issues with RtspMessage events try turning on ThreadEvents and you should be able to handle all rtsp messages regardless of frequency.

Let me know if you have further issues.
Marked as answer by juliusfriedman on 7/7/2016 at 6:30 PM