Re: [PATCH] fastboot: Fix overflow when calculating chunk size

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3136 bytes --]

On Fri, Apr 16, 2021 at 05:58:21PM -0400, Sean Anderson wrote:

> If a chunk was larger than 4GiB, then chunk_data_sz would overflow and
> blkcnt would not be calculated correctly. Upgrade it to a u64 and cast
> its multiplicands as well. Also fix bytes_written while we're at it.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> ---
> 
>  lib/image-sparse.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/image-sparse.c b/lib/image-sparse.c
> index 187ac28cd3..52c8dcc08c 100644
> --- a/lib/image-sparse.c
> +++ b/lib/image-sparse.c
> @@ -55,10 +55,10 @@ int write_sparse_image(struct sparse_storage *info,
>  	lbaint_t blk;
>  	lbaint_t blkcnt;
>  	lbaint_t blks;
> -	uint32_t bytes_written = 0;
> +	uint64_t bytes_written = 0;
>  	unsigned int chunk;
>  	unsigned int offset;
> -	unsigned int chunk_data_sz;
> +	uint64_t chunk_data_sz;
>  	uint32_t *fill_buf = NULL;
>  	uint32_t fill_val;
>  	sparse_header_t *sparse_header;
> @@ -132,7 +132,7 @@ int write_sparse_image(struct sparse_storage *info,
>  				 sizeof(chunk_header_t));
>  		}
>  
> -		chunk_data_sz = sparse_header->blk_sz * chunk_header->chunk_sz;
> +		chunk_data_sz = ((u64)sparse_header->blk_sz) * chunk_header->chunk_sz;
>  		blkcnt = chunk_data_sz / info->blksz;
>  		switch (chunk_header->chunk_type) {
>  		case CHUNK_TYPE_RAW:
> @@ -162,7 +162,7 @@ int write_sparse_image(struct sparse_storage *info,
>  				return -1;
>  			}
>  			blk += blks;
> -			bytes_written += blkcnt * info->blksz;
> +			bytes_written += ((u64)blkcnt) * info->blksz;
>  			total_blocks += chunk_header->chunk_sz;
>  			data += chunk_data_sz;
>  			break;
> @@ -222,7 +222,7 @@ int write_sparse_image(struct sparse_storage *info,
>  				blk += blks;
>  				i += j;
>  			}
> -			bytes_written += blkcnt * info->blksz;
> +			bytes_written += ((u64)blkcnt) * info->blksz;
>  			total_blocks += chunk_data_sz / sparse_header->blk_sz;
>  			free(fill_buf);
>  			break;
> @@ -253,7 +253,7 @@ int write_sparse_image(struct sparse_storage *info,
>  
>  	debug("Wrote %d blocks, expected to write %d blocks\n",
>  	      total_blocks, sparse_header->total_blks);
> -	printf("........ wrote %u bytes to '%s'\n", bytes_written, part_name);
> +	printf("........ wrote %llu bytes to '%s'\n", bytes_written, part_name);
>  
>  	if (total_blocks != sparse_header->total_blks) {
>  		info->mssg("sparse image write failure", response);

This results in things like:
            pico-dwarf-imx7d: all +506 bss +48 rodata +2 text +456
               u-boot: add: 1/0, grow: 1/0 bytes: 452/0 (452)
                 function                                   old     new   delta
                 __aeabi_uldivmod                             -     392    +392
                 write_sparse_image                         712     772     +60

Which I believe means that some of the division above needs to be
converted to use do_div().  Since I can't easily confirm the changes,
can you please check in to it?  Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]