Re: [PATCH] parse_tag_buffer(): do not prefixcmp() out of range

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

2011/2/16 Junio C Hamano <gitster@pobox.com>:
>> -     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@gmail.com> 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