Re: [PATCH v3] block: bugfix for Amiga partition overflow check patch

[Martin Steigerwald <martin@lichtvoll.de>]

Michael Schmitz - 04.07.23, 07:49:55 CEST:
> Making 'blk' sector_t (i.e. 64 bit if LBD support is active)
> fails the 'blk>0' test in the partition block loop if a
> value of (signed int) -1 is used to mark the end of the
> partition block list.
> 
> This bug was introduced in patch 3 of my prior Amiga partition
> support fixes series, and spotted by Christian Zigotzky when
> testing the latest block updates.
> 
> Explicitly cast 'blk' to signed int to allow use of -1 to
> terminate the partition block linked list.
> 
> Testing by Christian also exposed another aspect of the old
> bug fixed in commits fc3d092c6b ("block: fix signed int
> overflow in Amiga partition support") and b6f3f28f60
> ("block: add overflow checks for Amiga partition support"):
> 
> Partitions that did overflow the disk size (due to 32 bit int
> overflow) were not skipped but truncated to the end of the
> disk. Users who missed the warning message during boot would
> go on to create a filesystem with a size exceeding the
> actual partition size. Now that the 32 bit overflow has been
> corrected, such filesystems may refuse to mount with a
> 'filesystem exceeds partition size' error. Users should
> either correct the partition size, or resize the filesystem
> before attempting to boot a kernel with the RDB fixes in
> place.
> 
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Fixes: b6f3f28f60 ("block: add overflow checks for Amiga partition support")
> Message-ID: 024ce4fa-cc6d-50a2-9aae-3701d0ebf668@xenosoft.de
> Cc: <stable@vger.kernel.org> # 6.4
> Link: https://lore.kernel.org/r/024ce4fa-cc6d-50a2-9aae-3701d0ebf668@xenosoft.de
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> 
> --
> 
> Changes since v2:
> 
> Adrian Glaubitz:
> - fix typo in commit message
> 
> Changes since v1:
> 
> - corrected Fixes: tag
> - added Tested-by:
> - reworded commit message to describe filesystem partition
>   size mismatch problem
> ---
>  block/partitions/amiga.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/partitions/amiga.c b/block/partitions/amiga.c
> index ed222b9c901b..506921095412 100644
> --- a/block/partitions/amiga.c
> +++ b/block/partitions/amiga.c
> @@ -90,7 +90,7 @@ int amiga_partition(struct parsed_partitions *state)
>  	}
>  	blk = be32_to_cpu(rdb->rdb_PartitionList);
>  	put_dev_sector(sect);
> -	for (part = 1; blk>0 && part<=16; part++, put_dev_sector(sect)) {
> +	for (part = 1; (s32) blk>0 && part<=16; part++, put_dev_sector(sect)) {
>  		/* Read in terms partition table understands */
>  		if (check_mul_overflow(blk, (sector_t) blksize, &blk)) {
>  			pr_err("Dev %s: overflow calculating partition block %llu! Skipping partitions %u and beyond\n",
> 

Looks good. I do not consider myself a kernel developer, but patch
description and patch itself make sense to me.

Reviewed-By: Martin Steigerwald <martin@lichtvoll.de>

Ciao,
-- 
Martin