Re: [bug] MemBuf and using as string

From: Andres Kroonmaa <andre@dont-contact.us>
Date: Tue, 16 Apr 2002 09:28:30 +0300

 Yeah, my fault, should have mentioned that after a commit.
 It was test optimisation that I found to be stable enough
 that I left it in on our production boxes. I was seeing the
 issue you mentioned but never thought it was caused by me ;)

 I think we should define a way to avoid clearing, its especially
 notable with larger buffers. One thing that comes to mind
 mind is defining memPoolCalloc and/or memPoolMalloc in addition
 to memPoolAlloc, and make gradual transition. Opinions?

On 16 Apr 2002, at 2:39, Henrik Nordstrom <hno@squid-cache.org> wrote:

> Found why we haven't seen this before.. the old MemPools always cleared
> freed memory, but the chunked pools sometimes skip this.. there is the
> following condition:
>
> memPoolPush(MemPool * pool, void *obj)
> {
> void **Free;
> if ((pool->obj_size % 2048) != 0)
> memset(obj, 0, pool->obj_size);
>
>
> Now, having pools who don't waste time on clearing their freed content
> is a good thing, but it then needs to be defined, not guessed from the
> size.
>
> I will make memBufAppend() \0 terminate the data, and replace the
> condition above with a comment that we should figure out a way to avoid
> this on the pure data buffer pools..
>
> Regards
> Henrik
>
>
> Henrik Nordstrom wrote:
> >
> > There is several places in the code using MemBuf as a string blindly
> > assuming there is a \0 terminator after the data, but MemBufAppend()
> > apparently do not guarantee this...
> >
> > Identified so far:
> >
> > debugObj()
> > log_mime_hdrs
> >
> > but there probably is more..
> >
> > Now to the question: Should we audit the code to make sure it properly
> > deals with this by appending the \0 where required before using it as a
> > string with the slight risk of accidently introducing a \0 in output
> > data, or modify MemBuf to always \0 terminate the data outside of the
> > accounted data size?
> >
> > I am a bit surprised we haven't seen this before.. I detected it by
> > noticing there was HTML error content in cache.log on some "Forwarding
> > loop detected" error messages (debugObj).
> >
> > Regards
> > Henrik

------------------------------------
 Andres Kroonmaa <andre@online.ee>
 CTO, Microlink Online
 Tel: 6501 731, Fax: 6501 725
 Pärnu mnt. 158, Tallinn,
 11317 Estonia
Received on Tue Apr 16 2002 - 00:36:12 MDT

This archive was generated by hypermail pre-2.1.9 : Tue Dec 09 2003 - 16:15:10 MST