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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michael,

On Tue, Jul 4, 2023 at 9:30 PM Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
On 4/07/23 19:20, Geert Uytterhoeven wrote:
On Tue, Jul 4, 2023 at 7:50 AM Michael Schmitz <schmitzmic@xxxxxxxxx> 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.
That's the explanation for what this patch does.

The below is not directly related to that, so IMHO it does not
belong in the description of this patch.

Yes, I realize that. I had hoped that by way of the Fixes: tag, people
would be able to relate that comment to the correct commit. Might be a
little circuitous ...

We do not really have a way to record comments in git history
after the fact.  The best you can do is to reply to the email thread
where the patch was submitted.  When people follow the Link:
tag to the lore archive in the original commit, they can read any follow-ups.

Does lore pick up related patches through the In-Reply-To header? In
that case it would be easiest for me to to put this comment in a cover
letter to the bugfix patch.

Lore does not do that (b4 (the tool to download patch series from lore)
usually can link a series to its previous version, though).
New replies sent to a patch submission do end up in the right thread,
so any later comments (bug reports, Reviewed/Tested-by tags, ...)
can be found easily by following the Link: tag in the commit.

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
I am confused.  So before, the partition size as seen by Linux after
the truncation, was correct?

No, it was incorrect (though valid).

On a 2 TB disk, a partition of 1.3 TB at the end of the disk (but not
extending to the very end!) would trigger a overflow in the size
calculation:

sda: p4 size 18446744071956107760 extends beyond EOD,

Oh, so they were not "truncated to the end of the disk"?

That's only noted somewhere inside put_partition. The effective
partition size seen by the kernel and user tools is then that of a
partition extending to EOD (in Christian's case a full 8 GB more than
recorded in the partition table).

go on to create a filesystem with a size exceeding the
actual partition size. Now that the 32 bit overflow has been
But if Linux did see the correct size, mkfs would have used the correct
size, too, and the size in the recorded file system should be correct?

mkfs used what the old kernel code gave as partition size. That did
'seem' correct at that time, but after the overflow fixes (which prevent
other partition miscalculations, which in Martin's case caused
partitions to overlap), the partitions size is actually correct and
smaller than the filesystem size.

I have a hunch I don't explain myself very well.

;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux