Wireshark · Wireshark-dev: Re: [Wireshark-dev] New packet list - out of memory?

[didier <dgautheron@xxxxxxxx>]

Hi,
Le jeudi 08 octobre 2009 à 09:58 -0400, Jeff Morriss a écrit :
> didier wrote:
> > But are canaries used at all? In my understanding without
> > DEBUG_INTENSE_CANARY_CHECKS they are never checked and it's unset by
> > default.
> 
> Erm, emem_free_all() checks that the canaries haven't been corrupted:
> 
> >  if (memcmp(npc->canary_info->canary[i], canary, npc->canary_info->cmp_len[i]) != 0)
> >      g_error("Memory corrupted");
Missed this one, thanks.

Canary size is between 8 to 15 bytes so it can be store in 3 bits and
it's only accessed sequentially, right?  Thus we don't need to store a
pointer to it only the delta to the previous one. 

Currently we have on a 32 bits system:
EMEM_ALLOCS_PER_CHUNK 10MB/64 = 163,840 allocations/chunk and 163840*5
for the canaries list (819,200 bytes).

The worst case would be 1 byte allocation --> 15 bytes canary:
10MB/16 == 655,360 allocations   
With 2 bits for delta size (in bytes), 6 to 30 bits for delta (1 to 4
bytes) it needs 2+6+3 = 11 bits per allocation.

And because we can recompute the canary size from the allocation size we
only need 8 bits: 2 bits for the delta size and 6 to 30 bits for the
payload.

New canary list size: 655,360 Bytes vs 819,200 today and no limit on the
number of allocations per chunk.

For Andrew's capture, with canaries it would need 14 chunks and add
around 40MB vs 8 bytes aligned allocations. 

As allocation and testing are done sequentially I don't believe it would
be slower than the current scheme.

> 
> I fixed a bug a while ago where a dissector was writing past the end of 
> its se_alloc()'d memory:
> 
> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1513
> 
> I don't think we can/should turn off canaries in se_ allocations. 
> Instead we should create a new canary-less allocator.  (Not sure what 
> such a thing should be named, of course...)
Didier