Ethereal-dev: Re: [Ethereal-dev] Double-free tvb bug in HTTP dissector with gzi pdecompression
From: Jerry Talkington
| On Fri, May 07, 2004 at 03:15:33PM +0200, Biot Olivier wrote:
| > |From: Biot Olivier
| > |
| > ||From: Jerry Talkington
| > ||
| > ||Ok, I'm able to reproduce this. If you have gzip enabled, but
don't
| > ||have desegmentation enabled for HTTP or TCP, then the crash
occurs.
| > ||I'll fix this tonight after work.
| > |
| > |I have TCP and HTTP reassembly switched on. I tested with and
| > |without color filters. Maybe the bug is somewhere else, like
| > |in the pinfo->private_data portion?
| >
| > Forgot to mention: if I disable "uncompress HTTP entity bodies"
| > In the HTTP preferences, then the crash does not occur.
|
| Hmm, I wasn't able to reproduce this with that option unchecked, but
it
| happened reliably when I disabled desegmentation.
|
| Here's a patch to fix the crashes, by commenting out a couple of
| tvb_free()s. I'm not sure if that would cause a memory leak, or if
the
| tvbuffs are freed automatically when no longer needed. I've marked
them
| with XXX until I get some clarification on that.
When specifying tvb_new_subset() the subset is automatically linked to
the original tvbuffer's chain. Whenever the original chain will be
freed, the subsets will be freed too. Freeing a subset before freeing
the original tvb may result in a "hole" of the original tvb where
parts of the memory have been freed already.
| I also commented out the tvb_set_free_cb(uncompr_tvb, g_free); in
| tvbuff.c, since that seems to make the data sources in the byte
| inspection pane disappear. I've also marked that with XXX.
That's strange. The free() callback function should only be called
when the whole tvbuffer structure to which the other tvbuffers are
linked (including the real data ones). Maybe there's still a
tvb_free() call somewhere where it oughtn't be?
Or should we need to increment the usage count by hand here? I'll have
a closer look once I have compiled the whole lot again :)
| chunked_encoding_dissector() also had a minor display bug if
| desegmentation didn't wasn't enabled. I've fixed that also (it's
the
| only thing not marked with XXX ;)
I'll have a closer look once I see the patch ;)
Regards,
Olivier
