* [PATCH] fastboot: Fix overflow when calculating chunk size
@ 2021-04-16 21:58 Sean Anderson
2021-04-19 4:31 ` Heiko Schocher
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sean Anderson @ 2021-04-16 21:58 UTC (permalink / raw)
To: u-boot
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>
---
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);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] fastboot: Fix overflow when calculating chunk size
2021-04-16 21:58 [PATCH] fastboot: Fix overflow when calculating chunk size Sean Anderson
@ 2021-04-19 4:31 ` Heiko Schocher
2021-04-19 14:13 ` Sean Anderson
2021-05-13 15:54 ` Sean Anderson
2021-05-26 21:25 ` Tom Rini
2 siblings, 1 reply; 7+ messages in thread
From: Heiko Schocher @ 2021-04-19 4:31 UTC (permalink / raw)
To: u-boot
Hello Sean,
On 16.04.21 23:58, 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>
> ---
>
> lib/image-sparse.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Wow chunk size larger than 4G!
But patch seems valid, so
Reviewed-by: Heiko Schocher <hs@denx.de>
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] fastboot: Fix overflow when calculating chunk size
2021-04-19 4:31 ` Heiko Schocher
@ 2021-04-19 14:13 ` Sean Anderson
0 siblings, 0 replies; 7+ messages in thread
From: Sean Anderson @ 2021-04-19 14:13 UTC (permalink / raw)
To: u-boot
On 4/19/21 12:31 AM, Heiko Schocher wrote:
> Hello Sean,
>
> On 16.04.21 23:58, 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>
>> ---
>>
>> lib/image-sparse.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> Wow chunk size larger than 4G!
Normally filesystems like ext4 put data all around the disk (i.e. the
used-block bitmap). However, if you use a log filesystem (like F2FS),
then all the data is at the beginning of the disk. In that case, it is
fairly easy to end up with a 4GiB or greater "don't care" chunk taking
up most of the disk.
--Sean
>
> But patch seems valid, so
>
> Reviewed-by: Heiko Schocher <hs@denx.de>
>
> bye,
> Heiko
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] fastboot: Fix overflow when calculating chunk size
2021-04-16 21:58 [PATCH] fastboot: Fix overflow when calculating chunk size Sean Anderson
2021-04-19 4:31 ` Heiko Schocher
@ 2021-05-13 15:54 ` Sean Anderson
2021-05-25 15:45 ` Sean Anderson
2021-05-26 21:25 ` Tom Rini
2 siblings, 1 reply; 7+ messages in thread
From: Sean Anderson @ 2021-05-13 15:54 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
Can this make it into 2020.07? Thanks,
--Sean
On 4/16/21 5:58 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>
> ---
>
> 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);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fastboot: Fix overflow when calculating chunk size
2021-05-13 15:54 ` Sean Anderson
@ 2021-05-25 15:45 ` Sean Anderson
2021-05-26 7:12 ` Lukasz Majewski
0 siblings, 1 reply; 7+ messages in thread
From: Sean Anderson @ 2021-05-25 15:45 UTC (permalink / raw)
To: u-boot, Lukasz Majewski
Cc: Gary Bisson, Patrick Delaunay, Roman Kovalivskyi, Heiko Schocher,
Simon Glass, Tom Rini
On 5/13/21 11:54 AM, Sean Anderson wrote:
> Hi Lukasz,
>
> Can this make it into 2020.07? Thanks,
ping? Should Tom pick this up instead?
--Sean
>
> --Sean
>
> On 4/16/21 5:58 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>
>> ---
>>
>> 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);
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fastboot: Fix overflow when calculating chunk size
2021-05-25 15:45 ` Sean Anderson
@ 2021-05-26 7:12 ` Lukasz Majewski
0 siblings, 0 replies; 7+ messages in thread
From: Lukasz Majewski @ 2021-05-26 7:12 UTC (permalink / raw)
To: Sean Anderson
Cc: u-boot, Gary Bisson, Patrick Delaunay, Roman Kovalivskyi,
Heiko Schocher, Simon Glass, Tom Rini
[-- Attachment #1: Type: text/plain, Size: 3690 bytes --]
Hi Sean,
> On 5/13/21 11:54 AM, Sean Anderson wrote:
> > Hi Lukasz,
> >
> > Can this make it into 2020.07? Thanks,
>
> ping? Should Tom pick this up instead?
>
Yes, Tom please pick it up - as I will not prepare PR sooner than June.
> --Sean
>
> >
> > --Sean
> >
> > On 4/16/21 5:58 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>
> >> ---
> >>
> >> 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);
> >>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fastboot: Fix overflow when calculating chunk size
2021-04-16 21:58 [PATCH] fastboot: Fix overflow when calculating chunk size Sean Anderson
2021-04-19 4:31 ` Heiko Schocher
2021-05-13 15:54 ` Sean Anderson
@ 2021-05-26 21:25 ` Tom Rini
2 siblings, 0 replies; 7+ messages in thread
From: Tom Rini @ 2021-05-26 21:25 UTC (permalink / raw)
To: Sean Anderson
Cc: u-boot, Lukasz Majewski, Gary Bisson, Patrick Delaunay,
Roman Kovalivskyi, Heiko Schocher, Simon Glass
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-05-26 21:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16 21:58 [PATCH] fastboot: Fix overflow when calculating chunk size Sean Anderson
2021-04-19 4:31 ` Heiko Schocher
2021-04-19 14:13 ` Sean Anderson
2021-05-13 15:54 ` Sean Anderson
2021-05-25 15:45 ` Sean Anderson
2021-05-26 7:12 ` Lukasz Majewski
2021-05-26 21:25 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.