Re: [PATCH] Fix delta integer overflows
- From
Martin Koegler <martin.koegler@chello.at>
- Date
- Aug 8, 2017, 06:20 UTC
- Message-ID
- <20170808062047.GA4091@mail.zuhause>
- In-Reply-To
- <alpine.DEB.2.21.1.1708072136290.4271@virtualbox>
On Mon, Aug 07, 2017 at 09:39:12PM +0200, Johannes Schindelin wrote:
> If you want to work on data in memory, then size_t is the appropriate data > type. We already use it elsewhere. Let's use it here, too, without the > intermediate bump from the incorrect `int` to the equally incorrect > `long`.
I disagree with "We already use it elsewhere.". The whole delta code uses "unsigned long" - look at delta.h. Look at unpack-objects.c. Or cache.h. Or pack-objects.c. Or index-pack.c.
Other possible cases: git grep "unsigned long" |grep size
So the codebase still suggests, that "unsigned long" is the data type for storing object sizes.
I would be fine with resubmitting a patch using size_t/off_t for the touched parts - changing the whole core code is a too invasive change for a bug fix.
Regards, Martin
From d97a7810ff679dd939972c151bcf23c122cdc570 Mon Sep 17 00:00:00 2001 From: Martin Koegler <martin.koegler@chello.at> Date: Mon, 7 Aug 2017 20:00:30 +0200 Subject: [PATCH] Fix delta integer overflows
The current delta code produces incorrect pack objects for files > 4GB.
Signed-off-by: Martin Koegler <martin.koegler@chello.at> --- diff-delta.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/diff-delta.c b/diff-delta.c index 3797ce6..cd238c8 100644 --- a/diff-delta.c +++ b/diff-delta.c @@ -319,7 +319,9 @@ create_delta(const struct delta_index *index, const void *trg_buf, unsigned long trg_size, unsigned long *delta_size, unsigned long max_size) { - unsigned int i, outpos, outsize, moff, msize, val; + unsigned int i, val; + off_t outpos, moff; + size_t l, outsize, msize; int inscnt; const unsigned char *ref_data, *ref_top, *data, *top; unsigned char *out; @@ -336,20 +338,20 @@ create_delta(const struct delta_index *index, return NULL; /* store reference buffer size */ - i = index->src_size; - while (i >= 0x80) { - out[outpos++] = i | 0x80; - i >>= 7; + l = index->src_size; + while (l >= 0x80) { + out[outpos++] = l | 0x80; + l >>= 7; } - out[outpos++] = i; + out[outpos++] = l; /* store target buffer size */ - i = trg_size; - while (i >= 0x80) { - out[outpos++] = i | 0x80; - i >>= 7; + l = trg_size; + while (l >= 0x80) { + out[outpos++] = l | 0x80; + l >>= 7; } - out[outpos++] = i; + out[outpos++] = l; ref_data = index->src_buf; ref_top = ref_data + index->src_size;
-- 2.1.4