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

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

[Sean Anderson]

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>
---
sizes for pico-dwarf-imx7d are as follows:

   text    data     bss     dec     hex version
 431079   45114   38612  514805   7daf5 u-boot/master
 431529   45114   38612  515255   7dcb7 u-boot/V1
 431245   45114   38652  515011   7dbc3 u-boot/V2

so a total growth of around 200 bytes, down from 450.

Changes in v2:
- Converted divisions to use do_div

 lib/image-sparse.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index 187ac28cd3..d80fdbbf58 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,8 +132,8 @@ int write_sparse_image(struct sparse_storage *info,
 				 sizeof(chunk_header_t));
 		}
 
-		chunk_data_sz = sparse_header->blk_sz * chunk_header->chunk_sz;
-		blkcnt = chunk_data_sz / info->blksz;
+		chunk_data_sz = ((u64)sparse_header->blk_sz) * chunk_header->chunk_sz;
+		blkcnt = DIV_ROUND_UP_ULL(chunk_data_sz, info->blksz);
 		switch (chunk_header->chunk_type) {
 		case CHUNK_TYPE_RAW:
 			if (chunk_header->total_sz !=
@@ -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,8 +222,9 @@ int write_sparse_image(struct sparse_storage *info,
 				blk += blks;
 				i += j;
 			}
-			bytes_written += blkcnt * info->blksz;
-			total_blocks += chunk_data_sz / sparse_header->blk_sz;
+			bytes_written += ((u64)blkcnt) * info->blksz;
+			total_blocks += DIV_ROUND_UP_ULL(chunk_data_sz,
+							 sparse_header->blk_sz);
 			free(fill_buf);
 			break;
 
@@ -253,7 +254,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);
-- 
2.25.1

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

[Sean Anderson]

This should have

Reviewed-by: Heiko Schocher <hs@denx.de>

from v1, which I forgot to add.

On 5/27/21 12:02 PM, 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>
> ---
> sizes for pico-dwarf-imx7d are as follows:
> 
>     text    data     bss     dec     hex version
>   431079   45114   38612  514805   7daf5 u-boot/master
>   431529   45114   38612  515255   7dcb7 u-boot/V1
>   431245   45114   38652  515011   7dbc3 u-boot/V2
> 
> so a total growth of around 200 bytes, down from 450.
> 
> Changes in v2:
> - Converted divisions to use do_div
> 
>   lib/image-sparse.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/image-sparse.c b/lib/image-sparse.c
> index 187ac28cd3..d80fdbbf58 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,8 +132,8 @@ int write_sparse_image(struct sparse_storage *info,
>   				 sizeof(chunk_header_t));
>   		}
>   
> -		chunk_data_sz = sparse_header->blk_sz * chunk_header->chunk_sz;
> -		blkcnt = chunk_data_sz / info->blksz;
> +		chunk_data_sz = ((u64)sparse_header->blk_sz) * chunk_header->chunk_sz;
> +		blkcnt = DIV_ROUND_UP_ULL(chunk_data_sz, info->blksz);
>   		switch (chunk_header->chunk_type) {
>   		case CHUNK_TYPE_RAW:
>   			if (chunk_header->total_sz !=
> @@ -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,8 +222,9 @@ int write_sparse_image(struct sparse_storage *info,
>   				blk += blks;
>   				i += j;
>   			}
> -			bytes_written += blkcnt * info->blksz;
> -			total_blocks += chunk_data_sz / sparse_header->blk_sz;
> +			bytes_written += ((u64)blkcnt) * info->blksz;
> +			total_blocks += DIV_ROUND_UP_ULL(chunk_data_sz,
> +							 sparse_header->blk_sz);
>   			free(fill_buf);
>   			break;
>   
> @@ -253,7 +254,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);
> 

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

[Tom Rini]

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

On Thu, May 27, 2021 at 12:02:34PM -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>

Applied to u-boot/master, thanks!

-- 
Tom

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