Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
- From
Torsten Bögershausen <tboegi@web.de>
- Date
- May 5, 2026, 19:11 UTC
- Message-ID
- <20260505191100.GA12275@tb-raspi4>
- In-Reply-To
- <dc660106ea8511e6adc44d2b70e9a4ae8b18090e.1777914508.git.gitgitgadget@gmail.com>
On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
Show 52 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When unpacking objects from a packfile, the object size is decoded
> from a variable-length encoding. On platforms where unsigned long is
> 32-bit (such as Windows, even in 64-bit builds), the shift operation
> overflows when decoding sizes larger than 4GB. The result is a
> truncated size value, causing the unpacked object to be corrupted or
> rejected.
>
> Fix this by changing the size variable to size_t, which is 64-bit on
> 64-bit platforms, and ensuring the shift arithmetic occurs in 64-bit
> space.
>
> This was originally authored by LordKiRon <https://github.com/LordKiRon>,
> who preferred not to reveal their real name and therefore agreed that I
> take over authorship.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/index-pack.c | 9 +++++----
> builtin/unpack-objects.c | 5 +++--
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index ca7784dc2c..cc660582e9 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -37,7 +37,7 @@ static const char index_pack_usage[] =
>
> struct object_entry {
> struct pack_idx_entry idx;
> - unsigned long size;
> + size_t size;
> unsigned char hdr_size;
> signed char type;
> signed char real_type;
> @@ -469,7 +469,7 @@ static int is_delta_type(enum object_type type)
> return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
> }
>
> -static void *unpack_entry_data(off_t offset, unsigned long size,
> +static void *unpack_entry_data(off_t offset, size_t size,
> enum object_type type, struct object_id *oid)
> {
> static char fixed_buf[8192];
> @@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
> struct object_id *oid)
> {
> unsigned char *p;
> - unsigned long size, c;
> + size_t size;
> + unsigned long c;Does this look a little bit strange ? p points to an unsigned char (better would be *uint8_t) then it is dereferenced into an "unsigned long". Then it is masked with 0x7f In short: should "c" be declared as uint8_t ?
Show 39 quoted lines
> off_t base_offset;
> unsigned shift;
> void *data;
> @@ -542,7 +543,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
> p = fill(1);
> c = *p;
> use(1);
> - size += (c & 0x7f) << shift;
> + size += ((size_t)c & 0x7f) << shift;
> shift += 7;
> }
> obj->size = size;
> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index e01cf6e360..59a36c2481 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -533,7 +533,8 @@ static void unpack_one(unsigned nr)
> {
> unsigned shift;
> unsigned char *pack;
> - unsigned long size, c;
> + size_t size;
> + unsigned long c;
> enum object_type type;
>
> obj_list[nr].offset = consumed_bytes;
> @@ -548,7 +549,7 @@ static void unpack_one(unsigned nr)
> pack = fill(1);
> c = *pack;
> use(1);
> - size += (c & 0x7f) << shift;
> + size += ((size_t)c & 0x7f) << shift;
> shift += 7;
> }
>
> --
> gitgitgadget
>
>