Re: [PATCH v3] pack-objects: use size_t for streamed large blobs

[Junio C Hamano <gitster@pobox.com>]

Elia Pinto <gitter.spiros@gmail.com> writes:

> 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.

Makes sense.

Will queue.  Thanks.

> -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