[reSIProcate] resiprocate crash due to no free memory when receiving sip message with large header

[Byron Campen]

	If you delete mBuffer and mMessage first, you'll have a much better  
chance at being able to log, I bet.

Best regards,
Byron Campen

> Markus;
> What platform are you running on? Your patch isn't really reliable..  
> once you have a new allocation failure, certain assurances from the  
> environment are no longer preset and even running ErrLog() is a  
> problem...
>
> Alan
>
> On Jul 11, 2008, at 03:00 , Markus Eriksson wrote:
>
>> 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 at estacado.net]
>> Sent: Thu 7/10/2008 10:42 PM
>> To: Alan Hawrylyshen
>> Cc: Markus Eriksson; resiprocate-devel at resiprocate.or
>> 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...
>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20080711/00c84adb/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2423 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20080711/00c84adb/attachment.bin>