On 06/07/2012 10:43 PM, Branko Čibej wrote:
On 07.06.2012 12:16, Daniel Widenfalk wrote:
On 2012-06-07 11:47, Bert Huijben wrote:
-----Original Message-----
From: Bert Huijben [mailto:b...@qqmail.nl]
Sent: donderdag 7 juni 2012 11:34
To: 'Daniel Widenfalk'; dev@subversion.apache.org;
us...@subversion.apache.org
Subject: RE: Potential issue in
libsvn_diff:diff_file.c:find_identical_prefix
-----Original Message-----
From: Daniel Widenfalk [mailto:daniel.widenf...@iar.se]
Sent: donderdag 7 juni 2012 11:06
To: dev@subversion.apache.org; us...@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.
Unless the platform ABI is totally broken, then it should always be the
case that sizeof(uintptr_t)>= sizeof(size_t), and the same holds for
the APR aliases; assuming of course that they're defined correctly. :)
r1348472 should eliminate that inconsistency.
-- Stefan^2.
