Re: [PATCH] fix diff-delta bad memory access

From: Linus Torvalds <torvalds@osdl.org>
Date: May 10, 2006, 17:00 UTC
Message-ID: <Pine.LNX.4.64.0605100953090.3718@g5.osdl.org>
In-Reply-To: <Pine.LNX.4.64.0605101216360.24505@localhost.localdomain>

On Wed, 10 May 2006, Nicolas Pitre wrote:

>
> It cannot be assumed that the given buffer will never be moved when 
> shrinking the allocated memory size with realloc().  So let's ignore 
> that optimization for now.

Yeah, that was totally bogus. It would re-allocate _part_ of an allocation, but that allocation contained not just the index, but all the hashes and the hash entries too!

This has nothing to do with moving a buffer - it has everything to do with the fact that you shrunk a buffer that still contained all the other buffers: you shrunk the "mem" allocation to fit just the first part of it, and entirely ignored the "hash" and "entry" parts of it.

Btw, I think that whole "allocate everything in one allocation" thing is potentially bogus even the way it is now, if the alignment constraints of "index", "hash" and "entry" are different.

When you do

	..
	index = mem;
	mem = index + 1;
	hash = mem;
	mem = hash + hsize;
	entry = mem;
	..

it's perfectly fine for "index", but "hash" and "entry" end up having alignments that depend on the size/alignment of "index" (and for "entry" on "hash").

So if their alignment requirements are different, you're basically screwed.

It may work in practice (maybe they all align on pointer boundaries), but it's damn scary. You should re-consider, or at least make that code be a lot safer (like actually taking alignment into consideration, both for total size and for the offset calculations).

That could be done by using unions or something.

			Linus