* [PATCH] pack-objects: use size_t for streamed large blobs @ 2026-10-31 18:04 Elia Pinto 2026-10-31 18:37 ` [PATCH v2] " Elia Pinto 0 siblings, 1 reply; 4+ messages in thread From: Elia Pinto @ 2026-10-31 18:04 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Elia Pinto, Olamide Caleb Bello, Junio C Hamano, Taylor Blau, Patrick Steinhardt In builtin/pack-objects.c, write_no_reuse_object() handles blobs exceeding core.bigFileThreshold by writing them in a streaming path. It takes the size from st->size (size_t) and assigns it to an "unsigned long" variable, which is then written to the pack object header, and write_large_blob_data() adds up the compressed length in an "unsigned long" as well. On platforms where "long" is 32 bits, handling a blob of 4GiB or more truncates the size in the pack header while the data decompresses to the full size, corrupting the object. If the compressed data also exceeds 4GiB, the length accumulation overflows "unsigned long", misaligning the offsets of all subsequent objects in the index. When running repack or gc on a loose large blob, the original loose copy is pruned, leaving only the corrupted packed copy and resulting in data loss. Truncating st->size to 16 bits, with a 100KiB loose blob reachable from a commit and "git -c core.bigFileThreshold=1k repack -a -d", leaves a corrupt packed object and no loose copy. Switch the streamed path to use size_t for size and datalen, make write_large_blob_data() add up and return the length as a size_t, and use off_t for write_no_reuse_object()'s return value. Other truncations in pack-objects are not reachable because core.bigFileThreshold is itself an "unsigned long". Triggering this requires processing an object of 4GiB or more on a platform where "long" is 32 bits, so no test is added. Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> --- builtin/pack-objects.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index ca3a891dfb..452e785888 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -412,13 +412,13 @@ static unsigned long do_compress(void **pptr, unsigned long size) return stream.total_out; } -static unsigned long write_large_blob_data(struct odb_stream *st, struct hashfile *f, - const struct object_id *oid) +static size_t write_large_blob_data(struct odb_stream *st, struct hashfile *f, + const struct object_id *oid) { git_zstream stream; unsigned char ibuf[1024 * 16]; unsigned char obuf[1024 * 16]; - unsigned long olen = 0; + size_t olen = 0; struct repo_config_values *cfg = repo_config_values(the_repository); git_deflate_init(&stream, cfg->pack_compression_level); @@ -513,10 +513,10 @@ static inline int oe_size_greater_than(struct packing_data *pack, } /* Return 0 if we will bust the pack-size limit */ -static unsigned long write_no_reuse_object(struct hashfile *f, struct object_entry *entry, - unsigned long limit, int usable_delta) +static off_t write_no_reuse_object(struct hashfile *f, struct object_entry *entry, + unsigned long limit, int usable_delta) { - unsigned long size, datalen; + size_t size, datalen; unsigned char header[MAX_PACK_OBJECT_HEADER], dheader[MAX_PACK_OBJECT_HEADER]; unsigned hdrlen; base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7 -- 2.53.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2] pack-objects: use size_t for streamed large blobs 2026-10-31 18:04 [PATCH] pack-objects: use size_t for streamed large blobs Elia Pinto @ 2026-10-31 18:37 ` Elia Pinto 2026-11-01 6:37 ` Junio C Hamano 2026-11-01 7:29 ` [PATCH v3] " Elia Pinto 0 siblings, 2 replies; 4+ messages in thread From: Elia Pinto @ 2026-10-31 18:37 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Elia Pinto, Simon Richter, Patrick Steinhardt, Junio C Hamano, Olamide Caleb Bello, Taylor Blau In builtin/pack-objects.c, write_no_reuse_object() handles blobs exceeding core.bigFileThreshold by writing them in a streaming path. It takes the size from st->size (size_t) and assigns it to an "unsigned long" variable, which is then written to the pack object header, and write_large_blob_data() adds up the compressed length in an "unsigned long" as well. On platforms where "long" is 32 bits, handling a blob of 4GiB or more truncates the size in the pack header while the data decompresses to the full size, corrupting the object. If the compressed data also exceeds 4GiB, the length accumulation overflows "unsigned long", misaligning the offsets of all subsequent objects in the index. When running repack or gc on a loose large blob, the original loose copy is pruned, leaving only the corrupted packed copy and resulting in data loss. Truncating st->size to 16 bits, with a 100KiB loose blob reachable from a commit and "git -c core.bigFileThreshold=1k repack -a -d", leaves a corrupt packed object and no loose copy. Switch the streamed path to use size_t for the object size, and off_t for the length of the compressed data, which write_large_blob_data() adds up while it writes the stream piece by piece, and for write_no_reuse_object()'s return value, like write_reuse_object() already does. Other truncations in pack-objects are not reachable because core.bigFileThreshold is itself an "unsigned long". Triggering this requires processing an object of 4GiB or more on a platform where "long" is 32 bits, so no test is added. Helped-by: Simon Richter <Simon.Richter@hogyros.de> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> --- Changes since v1: use off_t for the length of the streamed data, following Simon's comment. builtin/pack-objects.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index ca3a891dfb..24203b5359 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -412,13 +412,13 @@ static unsigned long do_compress(void **pptr, unsigned long size) return stream.total_out; } -static unsigned long write_large_blob_data(struct odb_stream *st, struct hashfile *f, - const struct object_id *oid) +static off_t write_large_blob_data(struct odb_stream *st, struct hashfile *f, + const struct object_id *oid) { git_zstream stream; unsigned char ibuf[1024 * 16]; unsigned char obuf[1024 * 16]; - unsigned long olen = 0; + off_t olen = 0; struct repo_config_values *cfg = repo_config_values(the_repository); git_deflate_init(&stream, cfg->pack_compression_level); @@ -513,10 +513,11 @@ static inline int oe_size_greater_than(struct packing_data *pack, } /* Return 0 if we will bust the pack-size limit */ -static unsigned long write_no_reuse_object(struct hashfile *f, struct object_entry *entry, - unsigned long limit, int usable_delta) +static off_t write_no_reuse_object(struct hashfile *f, struct object_entry *entry, + unsigned long limit, int usable_delta) { - unsigned long size, datalen; + size_t size; + off_t datalen; unsigned char header[MAX_PACK_OBJECT_HEADER], dheader[MAX_PACK_OBJECT_HEADER]; unsigned hdrlen; Range-diff against v1: 1: 45e45c5b75 ! 1: 185b9da891 pack-objects: use size_t for streamed large blobs @@ Commit message from a commit and "git -c core.bigFileThreshold=1k repack -a -d", leaves a corrupt packed object and no loose copy. - Switch the streamed path to use size_t for size and datalen, make - write_large_blob_data() add up and return the length as a size_t, and - use off_t for write_no_reuse_object()'s return value. Other truncations - in pack-objects are not reachable because core.bigFileThreshold is - itself an "unsigned long". + Switch the streamed path to use size_t for the object size, and off_t + for the length of the compressed data, which write_large_blob_data() + adds up while it writes the stream piece by piece, and for + write_no_reuse_object()'s return value, like write_reuse_object() + already does. Other truncations in pack-objects are not reachable + because core.bigFileThreshold is itself an "unsigned long". Triggering this requires processing an object of 4GiB or more on a platform where "long" is 32 bits, so no test is added. + Helped-by: Simon Richter <Simon.Richter@hogyros.de> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> ## builtin/pack-objects.c ## @@ builtin/pack-objects.c: static unsigned long do_compress(void **pptr, unsigned l -static unsigned long write_large_blob_data(struct odb_stream *st, struct hashfile *f, - const struct object_id *oid) -+static size_t write_large_blob_data(struct odb_stream *st, struct hashfile *f, -+ const struct object_id *oid) ++static off_t write_large_blob_data(struct odb_stream *st, struct hashfile *f, ++ const struct object_id *oid) { git_zstream stream; unsigned char ibuf[1024 * 16]; unsigned char obuf[1024 * 16]; - unsigned long olen = 0; -+ size_t olen = 0; ++ off_t olen = 0; struct repo_config_values *cfg = repo_config_values(the_repository); git_deflate_init(&stream, cfg->pack_compression_level); @@ builtin/pack-objects.c: static inline int oe_size_greater_than(struct packing_da + unsigned long limit, int usable_delta) { - unsigned long size, datalen; -+ size_t size, datalen; ++ size_t size; ++ off_t datalen; unsigned char header[MAX_PACK_OBJECT_HEADER], dheader[MAX_PACK_OBJECT_HEADER]; unsigned hdrlen; base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7 -- 2.53.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] pack-objects: use size_t for streamed large blobs 2026-10-31 18:37 ` [PATCH v2] " Elia Pinto @ 2026-11-01 6:37 ` Junio C Hamano 2026-11-01 7:29 ` [PATCH v3] " Elia Pinto 1 sibling, 0 replies; 4+ messages in thread From: Junio C Hamano @ 2026-11-01 6:37 UTC (permalink / raw) To: Elia Pinto Cc: git, Johannes Schindelin, Simon Richter, Patrick Steinhardt, Olamide Caleb Bello, Taylor Blau Elia Pinto <gitter.spiros@gmail.com> writes: > In builtin/pack-objects.c, write_no_reuse_object() handles blobs > exceeding core.bigFileThreshold by writing them in a streaming path. It > takes the size from st->size (size_t) and assigns it to an "unsigned > long" variable, which is then written to the pack object header, and > write_large_blob_data() adds up the compressed length in an "unsigned > long" as well. > > On platforms where "long" is 32 bits, handling a blob of 4GiB or more On 32-bit boxes, replacing ulong with size_t would not fix anything, which you already had an discussion. Please rewrite the above to stress that you are talking about platforms where "long" is 32 bits but "size_t" is larger than that, so that the next person do not have to be puzzled by thinking this is about 32-bit boxes. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3] pack-objects: use size_t for streamed large blobs 2026-10-31 18:37 ` [PATCH v2] " Elia Pinto 2026-11-01 6:37 ` Junio C Hamano @ 2026-11-01 7:29 ` Elia Pinto 1 sibling, 0 replies; 4+ messages in thread From: Elia Pinto @ 2026-11-01 7:29 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Elia Pinto, Simon Richter, Taylor Blau, Junio C Hamano, Olamide Caleb Bello, Patrick Steinhardt In builtin/pack-objects.c, write_no_reuse_object() handles blobs exceeding core.bigFileThreshold by writing them in a streaming path. It takes the size from st->size (size_t) and assigns it to an "unsigned long" variable, which is then written to the pack object header, and write_large_blob_data() adds up the compressed length in an "unsigned long" as well. On platforms where "long" is 32 bits but size_t is 64 bits, like 64-bit Windows, handling a blob of 4GiB or more truncates the size in the pack header while the data decompresses to the full size, corrupting the object. If the compressed data also exceeds 4GiB, the length accumulation overflows "unsigned long", misaligning the offsets of all subsequent objects in the index. When running repack or gc on a loose large blob, the original loose copy is pruned, leaving only the corrupted packed copy and resulting in data loss. Truncating st->size to 16 bits, with a 100KiB loose blob reachable from a commit and "git -c core.bigFileThreshold=1k repack -a -d", leaves a corrupt packed object and no loose copy. Switch the streamed path to use size_t for the object size, and off_t for the length of the compressed data, which write_large_blob_data() adds up while it writes the stream piece by piece, and for write_no_reuse_object()'s return value, like write_reuse_object() already does. Other truncations in pack-objects are not reachable because core.bigFileThreshold is itself an "unsigned long". Triggering this requires processing an object of 4GiB or more on a platform where "long" is 32 bits but size_t is 64 bits, so no test is added. Helped-by: Simon Richter <Simon.Richter@hogyros.de> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> --- Changes since v2: say that this is about platforms where "long" is 32 bits but size_t is 64 bits, as Junio asked. builtin/pack-objects.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index ca3a891dfb..24203b5359 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -412,13 +412,13 @@ static unsigned long do_compress(void **pptr, unsigned long size) return stream.total_out; } -static unsigned long write_large_blob_data(struct odb_stream *st, struct hashfile *f, - const struct object_id *oid) +static off_t write_large_blob_data(struct odb_stream *st, struct hashfile *f, + const struct object_id *oid) { git_zstream stream; unsigned char ibuf[1024 * 16]; unsigned char obuf[1024 * 16]; - unsigned long olen = 0; + off_t olen = 0; struct repo_config_values *cfg = repo_config_values(the_repository); git_deflate_init(&stream, cfg->pack_compression_level); @@ -513,10 +513,11 @@ static inline int oe_size_greater_than(struct packing_data *pack, } /* Return 0 if we will bust the pack-size limit */ -static unsigned long write_no_reuse_object(struct hashfile *f, struct object_entry *entry, - unsigned long limit, int usable_delta) +static off_t write_no_reuse_object(struct hashfile *f, struct object_entry *entry, + unsigned long limit, int usable_delta) { - unsigned long size, datalen; + size_t size; + off_t datalen; unsigned char header[MAX_PACK_OBJECT_HEADER], dheader[MAX_PACK_OBJECT_HEADER]; unsigned hdrlen; Range-diff against v2: 1: 185b9da891 ! 1: 02e682725b pack-objects: use size_t for streamed large blobs @@ Commit message write_large_blob_data() adds up the compressed length in an "unsigned long" as well. - On platforms where "long" is 32 bits, handling a blob of 4GiB or more - truncates the size in the pack header while the data decompresses to the - full size, corrupting the object. If the compressed data also exceeds - 4GiB, the length accumulation overflows "unsigned long", misaligning the - offsets of all subsequent objects in the index. + On platforms where "long" is 32 bits but size_t is 64 bits, like 64-bit + Windows, handling a blob of 4GiB or more truncates the size in the pack + header while the data decompresses to the full size, corrupting the + object. If the compressed data also exceeds 4GiB, the length + accumulation overflows "unsigned long", misaligning the offsets of all + subsequent objects in the index. When running repack or gc on a loose large blob, the original loose copy is pruned, leaving only the corrupted packed copy and resulting in data @@ Commit message because core.bigFileThreshold is itself an "unsigned long". Triggering this requires processing an object of 4GiB or more on a - platform where "long" is 32 bits, so no test is added. + platform where "long" is 32 bits but size_t is 64 bits, so no test is + added. Helped-by: Simon Richter <Simon.Richter@hogyros.de> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7 -- 2.53.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-01 7:29 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-31 18:04 [PATCH] pack-objects: use size_t for streamed large blobs Elia Pinto 2026-10-31 18:37 ` [PATCH v2] " Elia Pinto 2026-11-01 6:37 ` Junio C Hamano 2026-11-01 7:29 ` [PATCH v3] " Elia Pinto
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox