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 --- c/fast-import.c +++ w/fast-import.c @@ -554,6 +554,10 @@ static void *pool_alloc(size_t len) struct mem_pool *p; void *r; + /* round up to a 'uintmax_t' alignment */ + if (len & (sizeof(uintmax_t) - 1)) + len += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1)); + for (p = mem_pool; p; p = p->next_pool) if ((p->end - p->next_free >= len)) break; @@ -572,9 +576,6 @@ static void *pool_alloc(size_t len) } r = p->next_free; - /* round out to a 'uintmax_t' alignment */ - if (len & (sizeof(uintmax_t) - 1)) - len += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1)); p->next_free += len; return r; } -- 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
