On 2012-06-07 11:47, Bert Huijben wrote:
>
>
>> -----Original Message-----
>> From: Bert Huijben [mailto:bert_at_qqmail.nl]
>> Sent: donderdag 7 juni 2012 11:34
>> To: 'Daniel Widenfalk'; dev_at_subversion.apache.org;
>> users_at_subversion.apache.org
>> Subject: RE: Potential issue in
> libsvn_diff:diff_file.c:find_identical_prefix
>>
>>
>>
>>> -----Original Message-----
>>> From: Daniel Widenfalk [mailto:Daniel.Widenfalk_at_iar.se]
>>> Sent: donderdag 7 juni 2012 11:06
>>> To: dev_at_subversion.apache.org; users_at_subversion.apache.org
>>> Subject: Potential issue in
> libsvn_diff:diff_file.c:find_identical_prefix
>>>
>>> Hi,
>>>
>>> First off: I'm sorry if I post this in the wrong way.
>>>
>>> I've found a potential issue in the function "find_identical_prefix"
>>> in libsvn_diff/diff_file.c
>>>
>>> The faulty code looks like this:
>>>
>>> diff_file.c:432 (as per 1.7.1, code identical to 1.7.5)
>>> -------------------------------
>>> is_match = TRUE;
>>> for (delta = 0; delta < max_delta && is_match; delta +=
>>> sizeof(apr_uintptr_t))
>>> {
>>> apr_uintptr_t chunk = *(const apr_size_t *)(file[0].curp +
>> delta);
>>> if (contains_eol(chunk))
>>> break;
>>>
>>> for (i = 1; i < file_len; i++)
>>> if (chunk != *(const apr_size_t *)(file[i].curp + delta))
>>> {
>>> is_match = FALSE;
>>> delta -= sizeof(apr_size_t);
>>> break;
>>> }
>>> }
>>> -------------------------------
>>>
>>> The problem is that the 64-bit build I'm using (ColabNet) have
>>> different sizes for apr_uintptr_t and apr_size_t.
>>>
>>> From looking at the disassembly I can deduce that
>>> sizeof(apr_uintptr_t) = 4 and sizeof(apr_size_t) = 8. This leads
>>> to these two issues:
>>>
>>> 1) Data is truncated in the initial read-up to "chunk" and the compare
>>> in the inner loop will fail because the second read-up will compare
>>> a 64-bit value against a 32-bit value.
>>>
>>> 2) When the test fails it will back up delta by 8, not 4, resulting in
>>> a buffer advance of -4 later in the code. Once the search code has
>>> advanced another 4 character if will be back at the same spot.
>>>
>>> Rinse and repeat.
>>>
>>> Are these a known issues?
>>>
>>> In my case this results in an infinite loop on the following input
>>> string:
>>>
>>> 23 0a 23 20 54 68 69 73 20 70 72 6f 6a 65 63
>>>
>>> I found this out when my svn-client spiraled into an infinite loop
>>> and would not respond to ctrl-c during a "svn up" command.
>>
>> Which apr version did you use?
>>
>> I think this issue was fixed in Subversion 1.7.2:
>>
>> (From http://svn.apache.org/repos/asf/subversion/tags/1.7.2/CHANGES)
>> * properly define WIN64 on Windows x64 builds (r1188609)
>>
>> Not by a code change here in this piece of the code, but by making sure
> that
>> the pointer size is defined correctly in apr.
>> So that would fix this issue and maybe several others.
>
> And since then then also APR was patched to properly check for _WIN64
> instead of WIN64 for defining these variable types correctly.
Updating to 1.7.5 makes the issue go away. The type mismatch is still
in the 1.7.5 source but if the system do guarantee that
sizeof(apr_uintptr_t) == sizeof(apr_size_t)
then it should be ok.
I did try to look in the libsvn_diff bug tracker before posting but
didn't see anything there. Looking at build issues didn't occur to
me, even though it should have been obvious from the error.
Regards
/Daniel Widenfalk
Received on 2012-06-07 12:17:29 CEST