2011/2/16 Junio C Hamano <gitster@xxxxxxxxx>: >> - Â Â if (prefixcmp(bufptr, "tag ")) >> + Â Â if (bufptr + 4 < tail && !prefixcmp(bufptr, "tag ")) >> + Â Â Â Â Â Â ; Â Â Â Â Â Â Â /* good */ >> + Â Â else >> Â Â Â Â Â Â Â return -1; >> Â Â Â bufptr += 4; >> Â Â Â nl = memchr(bufptr, '\n', tail - bufptr); > > If there weren't enough bytes between bufptr and tail, prefixcmp may still > match with "tag " while later part of the matched string might be coming > from trailing garbage outside our memory. ÂUnless we correctly fail the > prefixcmp() part, memchr() would be fed negative value, no? Yes, memchr() would be fed negative, but prefixcmp() already steps outside allocated memory. I believe that caused valgrind error Thomas reported (although I couldn't reproduce it). > Nguyán ThÃi Ngác Duy <pclouds@xxxxxxxxx> writes: > >> There is a check (size < 64) at the beginning of the function, but >> that only covers object+type lines. >> >> Strictly speaking the current code is still correct even if it >> accesses outside 'data' because 'tail' is used right after >> prefixcmp() calls. > > What do you mean by this? I don't get it. Because memchr() would be fed negative, memchr() would fail so the code is still correct. -- Duy -- 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