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

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Christian,

Am 01.07.2023 um 18:40 schrieb Christian Zigotzky:
> On 01 July 2023 at 04:35 am, Michael Schmitz wrote:
>> 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.
>>
>> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
>> Fixes: b6f3f28f60 ("Linux 6.4")
>> 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>
>> ---
>>   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",
> Hello Michael,
>
> Thanks for your patch.
>
> I patched the latest git kernel source code with your patch today but
> unfortunately the kernel has reported a bad geometry. (EXT4-fs (sda4):
> bad geometry: block count ...)
>
> dmesg | grep -i sda
>
> [    4.025449] sd 0:0:0:0: [sda] 3907029168 512-byte logical blocks:
> (2.00 TB/1.82 TiB)
> [    4.071978] sd 0:0:0:0: [sda] 4096-byte physical blocks
> [    4.119333] sd 0:0:0:0: [sda] Write Protect is off
> [    4.165958] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
> [    4.212921] sd 0:0:0:0: [sda] Write cache: enabled, read cache:
> enabled, doesn't support DPO or FUA
> [    4.259469] sd 0:0:0:0: [sda] Preferred minimum I/O size 4096 bytes
> [    4.502519]  sda: RDSK (512) sda1 (DOS^G)(res 2 spb 2) sda2
> (SFS^B)(res 2 spb 1) sda3 (SFS^B)(res 2 spb 2) sda4 ((res 2 spb 1)

Good - all partitions are recognized again as they ought to be.

> [    4.551981] sd 0:0:0:0: [sda] Attached SCSI disk
> [   82.421727] EXT4-fs (sda4): bad geometry: block count 319655862
> exceeds size of device (317690430 blocks)

Now that may be a new bug... or just a filesystem created with incorrect 
assumptions about partition size.

That is the partition that had reported:

> sda: p4 size 18446744071956107760 extends beyond EOD, truncated

with my patches backed out? I wonder what size mkfs used when creating 
the filesystem?

The calculated size was clearly incorrect, but the truncated size may 
also be incorrect. The truncated size is likely that of a partition 
extending to the end of the disk, but your actual p4 size may have been 
smaller (your parted -l output had 1992GB which is 8 GB shorter than to 
EOD).

On the face of it, this does not look like a new bug in the RDB 
partition code, but I'd rather verify that by inspecting the RDB 
contents and carrying out the calculations by hand.

Can you please send a copy of the RDB (first few kB of the disk, 
something like dd if=/dev/sda of=rdb-sda.img bs=512 count=16 should do), 
and the output of cat /proc/partitions when running a kernel from before 
my RDB patches?

> I can't mount the ext4 partition on the RDB disk and booting isn't
> possible as well.

I suppose the ext4 filesystem must be resized to match the actual 
partition size. I don't think that can be done on a live, mounted 
filesystem. You may have to hook up the disk to another Linux host for 
that, and use resize2fs there (or boot a ramdisk containing that tool).

Cheers,

	Michael

>
> Thanks,
> Christian