Re: [PATCH] Fix delta integer overflows

Junio C Hamano [off-list ref] writes:

Martin Koegler [off-list ref] writes:

quoted

From: Martin Koegler <redacted>

The current delta code produces incorrect pack objects for files > 4GB.

Signed-off-by: Martin Koegler <redacted>
---
 diff-delta.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Just pass any file > 4 GB to the delta-compression [by increasing the delta limits].
As file size, a truncated 32bit value will be encoded, leading to broken pack files.

The patch obviously makes the code better and self consistent in
that "struct delta_index" has src_size as ulong, and this function
takes trg_size as ulong, and it was plain wrong for the code to
assume that "i", which is uint, can receive it safely.

In the longer term we might want to move to size_t or even
uintmax_t, as the ulong on a platform may not be long enough in
order to express the largest file size the platform can have, but
this patch (1) is good even without such a change, and (2) gives a
good foundation to build on if we want such a change on top.

Thanks.  Will queue.

Having said that, I am a bit curious how you came to this patch.
Was the issue found by code inspection, or did you actually have a
real life use case to raise the core.bigFileThreshold configuration
to a value above 4GB?

Thanks.

On Mon, Aug 07, 2017 at 06:44:16PM -0700, Junio C Hamano wrote:

Having said that, I am a bit curious how you came to this patch.
Was the issue found by code inspection, or did you actually have a
real life use case to raise the core.bigFileThreshold configuration
to a value above 4GB?

Real life use - tracking changes in larger files.

Raising the limit above 4GB suddenly resulted in a broken pack files in the repository and
aborts of various git commands.

Data is still recoverable with all size sanity checks disabled.

Regards,
Martin