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

[Alan Hawrylyshen]

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/a08ff9ee/attachment.htm>