rsync - Sep 2005 - [Bug 1529] 32bit rollover problem rsyncing files greater than 4GB in size

https://bugzilla.samba.org/show_bug.cgi?id=1529





------- Additional Comments From thomas@suse.de  2005-09-06 06:37 -------
Now when len is OFF_T is it possible that sum->count (which is size_t) in
sum_sizes_sqroot() will rollover too at line:
        sum->count      = (len + (blength - 1)) / blength; ?

When we assume all variables have all bits set:
2^64 + (2^32 - 1) / 2^32 = 2^32 + 1

But sum->count can only represent 2^32 and thereofre sum->count will be 0.

Did I made a mistake here?




-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=1529





------- Additional Comments From wayned@samba.org  2005-09-06 11:10 -------
(In reply to comment #8)
> Did I made a mistake here?
I have some minor quibbles with what you stated (which I will describe below),
but, as you point out, this value can theoretically overflow.  However, for the
overflow to happen the file must be nearly an exbibyte in size (over a billion
GiB), and anyone who is trying to rsync anywhere near that large of a file will
experience rsync grinding to a halt due to the hash collisions in its checksum
search algorithm (which will happen long before reaching rsync's maximum
file-size limit).

So, yes -- that value could theoretically overflow, but is not likely that it
will in actual practice.

As promised (threatened?? :-) ), I'll point out my quibbles with your logic,
just for completeness:

If we just ignore that 2^32+1 really would truncate to 1 instead of 0, the
remaining problems are due to size differences from the variables in the latest
rsync, which may stem from you looking at an older version of the source:

- "len" is an int64 whose maximum positive value is 2^63 - 1.
- "blength" has an enforced upper bound of 2^29.
- "count" is an int32 whose maximum positive value is 2^31 - 1.

The program logic as written (adding blength - 1 to the length) could cause a
signed value to overflow into the negative when it is near its limit, so,
let's
pretend the code is written like this (which it probably will be soon*):

    sum->remainder  = len % blength;
    sum->count      = len / blength + (sum->remainder != 0);

OK.  That makes the maximum result prior to truncation:

    (2^63-1) / (2^29) + 1 = 2^34

As noted before, this is larger than the 2^31 - 1 value that "count"
can hold.
I calculate that the maximum file size is therefore (2^31-1) * 2^29, or
1,152,921,504,069,976,064 bytes (or slightly less than 2^60, which is an
exbibyte).

* The reason I will probably change this to avoid the potential overflow of
adding (blength-1) is that rsync allows itself to be compiled on a system that
does not have 64-bit integers, which means that the "len" value might
really be
an int32, and we don't want the length to be able to overflow into negative
values.

-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=1529





------- Additional Comments From thomas@suse.de  2005-09-06 11:38 -------
thomas@spiral:~> cat rsync-intoverflowtest.c
#include <limits.h>
#include <stdio.h>

int main(void)
{
        unsigned i = UINT_MAX;

        printf("i = %u, i+1 = %u\n", i, i+1);

        exit(0);
}
thomas@spiral:~> ./rsync-intoverflowtest
i = 4294967295, i+1 = 0
thomas@spiral:~>

Yes, I looked at an older version (just the one we ship with our enterprise
server :).
I'll have a look at the rest of your comment tomorrow. :)

-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=1529





------- Additional Comments From thomas@suse.de  2005-09-07 01:36 -------
Sorry, Wayne you are right 2^32+1 really is 1. :)

Yes 2^60 seems much to big to be practicable today but maybe, to avoid problems
in the future, it would be better to have all this type-clean.

BTW, AFAICR int overflows for signed types depend on the compiler and are not
standardized. 

-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.