| < Previous by Date | Date Index | Next by Date > |
| < Previous in Thread | Thread Index | Next in Thread > |
It seems to be working fine.
/Markus
-----Original Message-----
From: Byron Campen [mailto:bcampen@xxxxxxxxxxxx]
Sent: Sat 7/12/2008 12:25 AM
To: Markus Eriksson
Cc: Alan Hawrylyshen; resiprocate-devel@xxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxx
> 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@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...
> >
> >
> >
>
>
>