Re: Subject: [PATCH RFC] block: fix Amiga RDB partition support for disks >= 2 TB

Michael Schmitz
2018-06-29T08:58:37Z

Hi Geert,

Am 28.06.18 um 18:45 schrieb Geert Uytterhoeven:
> Hi Michael,
>
> On Thu, Jun 28, 2018 at 6:59 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> Am 28.06.2018 um 09:20 schrieb Martin Steigerwald:
>>>>> And as stated in my other reply to the patch:
>>>>> partition needs 64 bit disk device support in AmigaOS or AmigaOS
>>>>> like
>>>>> operating systems (NSD64, TD64 or SCSI direct)
>>>> I'd probably leave it at 'disk needs 64 bit disk device support on
>>>> native OS', and only print that warning once.
>>> This is fine with me.
>> OK, I'll go with that.
> Do we really need the warning?
> Once the parsing is fixed doing 64-bit math, it does not matter for Linux
> anymore.
>
> Won't it make more sense to have the warning in the tool that created the
> partition table in the first place?
Sure, but we've seen one case of this in the wild, and the tool
apparently did not issue a warning.

I agree it's not an issue for Linux. If  you think dropping a warning
about something not actually relevant to Linux in the log would be
confusing, or if convention is to limit warnings strictly to behaviour
relevant to Linux, I can live without the warning. Joanne scared me a
bit about Amigoids angry at data loss, but I suppose there can't be many
around my neck of the woods.

>
>>> I would not name the kernel option "eat_my_rdb", but use a less
>>> dramatizing name.
>>>
>>> Maybe just: "allow_64bit_rdb" or something like that.
>> I don't expect to get away with that :-)
> I still fail to see what's the added value of the kernel option...
> Either the partition is usable, or not.

The question is - can writes to the disk cause any damage to data on the
disk, as seen by old OS versions? If the answer is no, we won't need the
option after all.

>>> How does the user come to know about this kernel option? Will you print
>>> its name in kernel log?
>> Depends on how easy we want to make it for users. If I put a BUG() trap
>> with the check, the resulting log section will point to a specific line
>> in block/partitions/amiga.c, from which the override option will be
>> obvious. But that might be a little too opaque for some...
> Please don't use BUG(), unless your goal is to attract attention (from
> Linus, who dislikes BUG()!).
I'd rather not abuse his patience. Thanks for the hint.
> Using BUG() would be a nice way to DoS someones machine by plugging in
> a USB stick with a malformed RDB.
I guess I deserved that. But BUG() doesn't panic now, does it? The ones
I saw did allow the kernel to happily carry on.

Cheers,

Michael

>
> Gr{oetje,eeting}s,
>
> Geert
>