Re: [PATCH v8 2/2] block: add overflow checks for Amiga partition support

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Christoph,

On 26/07/22 23:42, Christoph Hellwig wrote:
> On Tue, Jul 26, 2022 at 04:57:47PM +1200, Michael Schmitz wrote:
>> The Amiga partition parser module uses signed int for partition sector
>> address and count, which will overflow for disks larger than 1 TB.
>>
>> Use u64 as type for sector address and size to allow using disks up to
>> 2 TB without LBD support, and disks larger than 2 TB with LBD. The RBD
>> format allows to specify disk sizes up to 2^128 bytes (though native
>> OS limitations reduce this somewhat, to max 2^68 bytes), so check for
>> u64 overflow carefully to protect against overflowing sector_t.
>>
>> Bail out if sector addresses overflow 32 bits on kernels without LBD
>> support.
>>
>> This bug was reported originally in 2012, and the fix was created by
>> the RDB author, Joanne Dow <jdow@earthlink.net>. A patch had been
>> discussed and reviewed on linux-m68k at that time but never officially
>> submitted (now resubmitted as separate patch).
>> This patch adds additional error checking and warning messages.
>>
>> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=43511
>> Reported-by: Martin Steigerwald <Martin@lichtvoll.de>
>> Message-ID: <201206192146.09327.Martin@lichtvoll.de>
>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>>   block/partitions/amiga.c | 111 +++++++++++++++++++++++++++++++--------
>>   1 file changed, 89 insertions(+), 22 deletions(-)
>>
>> diff --git a/block/partitions/amiga.c b/block/partitions/amiga.c
>> index f98191545d9a..7356b39cbe10 100644
>> --- a/block/partitions/amiga.c
>> +++ b/block/partitions/amiga.c
>> @@ -11,10 +11,18 @@
>>   #define pr_fmt(fmt) fmt
>>   
>>   #include <linux/types.h>
>> +#include <linux/mm_types.h>
>> +#include <linux/overflow.h>
>>   #include <linux/affs_hardblocks.h>
>>   
>>   #include "check.h"
>>   
>> +/* magic offsets in partition DosEnvVec */
>> +#define NR_HD	3
>> +#define NR_SECT	5
>> +#define LO_CYL	9
>> +#define	HI_CYL	10
> The last line has a tab after the #define while the previous three
> don't.  Pick one style and stick to it for the others.
>
>>   		if (!data) {
>> -			pr_err("Dev %s: unable to read RDB block %d\n",
>> -			       state->disk->disk_name, blk);
>> +			pr_err("Dev %s: unable to read RDB block %llu\n",
>> +			       state->disk->disk_name, (u64) blk);
> No need for the various printk casts, a sector_t is always an
> unsigned long long.

Thanks, wil fix (and address the other issues raised).

Cheers,

     Michael