Re: [reSIProcate] resiprocate crash due to no free memory when receiving sip message with large header
Actually, this doesn't make any sense. The catch is the caller's responsibility. Never mind!
Best regards, Byron Campen I agree that we need some sort of configurable limit on header size. Also, I think it would be reasonable to put a catch inside MsgHeaderScanner::allocateBuffer(); this should provide a last line of defense against memory DoS shenanigans.
Best regards, Byron Campen That patch works great. 1+ MB headers are no longer a problem (on my system).
There still need to be some kind of handling for extremely large headers though. The problem that a tcp connection can use up all memory is still there, but this fixed the memory "leak".
Maybe the failed allocation should be detected and then just drop the connection? That would be good even if there is a limit on the header size.
Here is a patch that I believe will fix that issue also. It includes your patch from below and adds exception handling for when the memory allocation fails:
160a161
>
161d161
< mMessage->addBuffer(mBuffer);
163a164,197
>
> if(numUnprocessedChars==chunkLength)
> {
> // .bwc. MsgHeaderScanner wasn't able to parse anything useful;
> // don't bother mMessage yet, but make more room in mBuffer.
> size_t size = numUnprocessedChars*3/2;
> if (size < ConnectionBase::ChunkSize)
> {
> size = ConnectionBase::ChunkSize;
> }
> char* newBuffer;
> try{
> newBuffer = MsgHeaderScanner::allocateBuffer(size);
> }
> catch( ... )
> {
> ErrLog(<< "Out of memory during preparse! Discarding connection");
> delete [] mBuffer;
> delete mMessage;
> mMessage = 0;
> mBuffer = 0;
> delete this;
> return;
>
> }
> memcpy(newBuffer, unprocessedCharPtr, numUnprocessedChars);
> delete [] mBuffer;
> mBuffer = newBuffer;
> mBufferPos = numUnprocessedChars;
> mBufferSize = size;
> mConnState = ReadingHeaders;
> return;
> }
>
163a198
> mMessage->addBuffer(mBuffer);
184c219,234
< char* newBuffer = MsgHeaderScanner::allocateBuffer(size);
---
> char* newBuffer;
> try{
> newBuffer = MsgHeaderScanner::allocateBuffer(size);
> }
> catch( ... )
> {
> ErrLog(<< "Out of memory during preparse! Discarding connection");
> delete mMessage;
> mMessage = 0;
> mBuffer = 0;
> // !bwc! mMessage just took ownership of mBuffer, so we don't
> // delete it here. We do zero it though, for completeness.
> delete this;
> return;
>
> }
/Markus
-----Original Message-----
From: Byron Campen [mailto:bcampen@xxxxxxxxxxxx]
Sent: Thu 7/10/2008 10:42 PM
To: Alan Hawrylyshen
Cc: Markus Eriksson; resiprocate-devel@xxxxxxxxxxxxxx
Subject: Re: [reSIProcate] resiprocate crash due to no free memory when receiving sip message with large header
...
> I guess this would be pretty easy to implement; if scanChunk returns
> the whole buffer as unprocessed (ie; unprocessedCharPtr points at the
> beginning of mBuffer), we won't call addBuffer(). (We will make more
> room if necessary though) How's this for a patch:
> Index: ConnectionBase.cxx
> ===================================================================
> --- ConnectionBase.cxx (revision 8120)
> +++ ConnectionBase.cxx (working copy)
> @@ -158,9 +158,30 @@
> delete this;
> return;
> }
> - mMessage->addBuffer(mBuffer);
> +
> unsigned int numUnprocessedChars =
> (mBuffer + chunkLength) - unprocessedCharPtr;
> +
> + if(numUnprocessedChars==chunkLength)
> + {
> + // .bwc. MsgHeaderScanner wasn't able to parse anything useful;
> + // don't bother mMessage yet, but make more room in mBuffer.
> + size_t size = numUnprocessedChars*3/2;
> + if (size < ConnectionBase::ChunkSize)
> + {
> + size = ConnectionBase::ChunkSize;
> + }
> + char* newBuffer = MsgHeaderScanner::allocateBuffer(size);
> + memcpy(newBuffer, unprocessedCharPtr, numUnprocessedChars);
> + delete [] mBuffer;
> + mBuffer = newBuffer;
> + mBufferPos = numUnprocessedChars;
> + mBufferSize = size;
> + mConnState = ReadingHeaders;
> + return;
> + }
> +
> + mMessage->addBuffer(mBuffer);
> if (scanChunkResult == MsgHeaderScanner::scrNextChunk)
> {
> // Message header is incomplete...
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
|
Attachment:
smime.p7s
Description: S/MIME cryptographic signature