You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Daniel Widenfalk <Da...@iar.se> on 2012/06/07 11:06:25 UTC
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.
Regards
/Daniel Widenfalk
Re: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Posted by Stefan Fuhrmann <eq...@web.de>.
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:bert@qqmail.nl]
>>>> Sent: donderdag 7 juni 2012 11:34
>>>> To: 'Daniel Widenfalk'; dev@subversion.apache.org;
>>>> users@subversion.apache.org
>>>> Subject: RE: Potential issue in
>>> libsvn_diff:diff_file.c:find_identical_prefix
>>>>
>>>>> -----Original Message-----
>>>>> From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
>>>>> Sent: donderdag 7 juni 2012 11:06
>>>>> To: dev@subversion.apache.org; users@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.
Re: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Posted by Branko Čibej <br...@apache.org>.
On 07.06.2012 12:16, Daniel Widenfalk wrote:
> On 2012-06-07 11:47, Bert Huijben wrote:
>>
>>> -----Original Message-----
>>> From: Bert Huijben [mailto:bert@qqmail.nl]
>>> Sent: donderdag 7 juni 2012 11:34
>>> To: 'Daniel Widenfalk'; dev@subversion.apache.org;
>>> users@subversion.apache.org
>>> Subject: RE: Potential issue in
>> libsvn_diff:diff_file.c:find_identical_prefix
>>>
>>>
>>>> -----Original Message-----
>>>> From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
>>>> Sent: donderdag 7 juni 2012 11:06
>>>> To: dev@subversion.apache.org; users@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. :)
Since both types are unsigned by definition, standard C type coercion
rules guarantee that the uintptr_t result holds the same value as the
original size_t, so there isn't really any type mismatch here.
But I can't help wondering if that bit of code could be written more
elegantly without all the casts.
-- Brane
Re: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Posted by Daniel Widenfalk <Da...@iar.se>.
On 2012-06-07 11:47, Bert Huijben wrote:
>
>
>> -----Original Message-----
>> From: Bert Huijben [mailto:bert@qqmail.nl]
>> Sent: donderdag 7 juni 2012 11:34
>> To: 'Daniel Widenfalk'; dev@subversion.apache.org;
>> users@subversion.apache.org
>> Subject: RE: Potential issue in
> libsvn_diff:diff_file.c:find_identical_prefix
>>
>>
>>
>>> -----Original Message-----
>>> From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
>>> Sent: donderdag 7 juni 2012 11:06
>>> To: dev@subversion.apache.org; users@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
Re: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Posted by Daniel Widenfalk <Da...@iar.se>.
On 2012-06-07 11:47, Bert Huijben wrote:
>
>
>> -----Original Message-----
>> From: Bert Huijben [mailto:bert@qqmail.nl]
>> Sent: donderdag 7 juni 2012 11:34
>> To: 'Daniel Widenfalk'; dev@subversion.apache.org;
>> users@subversion.apache.org
>> Subject: RE: Potential issue in
> libsvn_diff:diff_file.c:find_identical_prefix
>>
>>
>>
>>> -----Original Message-----
>>> From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
>>> Sent: donderdag 7 juni 2012 11:06
>>> To: dev@subversion.apache.org; users@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
RE: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: donderdag 7 juni 2012 11:34
> To: 'Daniel Widenfalk'; dev@subversion.apache.org;
> users@subversion.apache.org
> Subject: RE: Potential issue in
libsvn_diff:diff_file.c:find_identical_prefix
>
>
>
> > -----Original Message-----
> > From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
> > Sent: donderdag 7 juni 2012 11:06
> > To: dev@subversion.apache.org; users@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.
Bert
>
> Bert
>
RE: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: donderdag 7 juni 2012 11:34
> To: 'Daniel Widenfalk'; dev@subversion.apache.org;
> users@subversion.apache.org
> Subject: RE: Potential issue in
libsvn_diff:diff_file.c:find_identical_prefix
>
>
>
> > -----Original Message-----
> > From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
> > Sent: donderdag 7 juni 2012 11:06
> > To: dev@subversion.apache.org; users@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.
Bert
>
> Bert
>
RE: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
> Sent: donderdag 7 juni 2012 11:06
> To: dev@subversion.apache.org; users@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.
Bert
RE: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
> Sent: donderdag 7 juni 2012 11:06
> To: dev@subversion.apache.org; users@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.
Bert