[reSIProcate] resiprocate crash due to no free memory when receiving sip message with large header
Byron Campen
bcampen at estacado.net
Mon Jul 14 10:40:07 CDT 2008
Excellent. Thanks for bringing this to our attention! I will be
cutting a point-release with this fix later today (after some testing
of my own).
Best regards,
Byron Campen
> It seems to be working fine.
>
> /Markus
>
>
> -----Original Message-----
> From: Byron Campen [mailto:bcampen at estacado.net]
> Sent: Sat 7/12/2008 12:25 AM
> To: Markus Eriksson
> Cc: Alan Hawrylyshen; resiprocate-devel at resiprocate.org
> Subject: Re: [reSIProcate] resiprocate crash due to no free memory
> when receiving sip message with large header
>
> Ok, I have committed the fix, plus some other fixes. Please
> take the
> code on head for a spin and see if it handles your testing.
>
> Best regards,
> Byron Campen
>
> > Yeah, I know that you cant guarantee anything there, and I also
> > later realized it would be better to do the ErrLog after the other
> > things have been deleted in case ErrLog also tries to allocate
> > memory. Since the logging might be different on different systems
> > you don't really know what it will do.
> > But my take on it is just that catching the exception is the only
> > reasonable thing you can do there. And most likely you will want it
> > logged somehow. If something like this happens and the system is no
> > longer stable I'd like to have the reason in the logs.
> >
> > Obviously, the best solution is to have checks in place so that you
> > will not even try the allocation, but catching the exception could/
> > should be an additional safe guard to prevent a complete crash.
> >
> > /Markus
> >
> >
> > -----Original Message-----
> > From: Alan Hawrylyshen on behalf of Alan Hawrylyshen
> > Sent: Fri 7/11/2008 3:43 PM
> > To: Markus Eriksson
> > Cc: Byron Campen; resiprocate-devel at resiprocate.org
> > Subject: Re: [reSIProcate] resiprocate crash due to no free memory
> > when receiving sip message with large header
> >
> > 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/20080714/1f555ff5/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/20080714/1f555ff5/attachment.bin>
More information about the resiprocate-devel
mailing list