On Sat, Dec 13, 2008 at 09:53:55PM -0800, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > >> As the `round out' takes place AFTER it found the room in the mem_pool, > >> there's a small chance of p->next_free being set outside of the chosen > >> area, up to (sizeof(uintmax_t) - 1) bytes. pool_strdup() is one of the > >> functions which can trigger the problem, when pool_alloc() finds a room > >> at the end of a pool entry and the requested length is not multiple of > >> size(uintmax_t). I believe attached patch addresses this problem. > > > > Thanks -- do you mean your reproducible crash does not reproduce with the > > patch anymore? > > > > I think your change to move the "round up" logic up in the codepath makes > > perfect sense. But your patch seems to conflate totally unrelated change > > to move memzero from the caller to callee into it, and I do not see the > > reason why it should be that way. If the caller asked 10 bytes to calloc > > from the pool, and the underlying pool allocator gives you a 16-byte > > block, you only have to guarantee that the first 10 bytes are cleared, and > > can leave the trailing padding 6 bytes at the end untouched. > > That is, something like this... > > fast-import.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > > diff --git c/fast-import.c w/fast-import.c > index 3c035a5..3276d5d 100644 [snip] Yes, that was what it originally looked like, but after fixing the crash I encountered another problem, and thought that I had to clear rounded up area too. It turned out that the second problem has nothing to do with [the pool allocator, but the data fed to git-fast-import. After fixing the data, the git-fast-import with above patch finished successfully, no more crashes. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
