Re: [PATCH 3/6] disk: part_mac: Fix stack corruption on devices with large blocks
Alexey Charkov <[email protected]>
| Newsgroups | org.u-boot-project.lists.u-boot |
|---|---|
| Message-ID | <CAKTNdwGmskh2+cLVmOGTmWrFhhGR=v4qF7zR=rmxScY6FOO55Q@mail.gmail.com> |
On Thu, Aug 27, 2026 at 12:25 PM Heinrich Schuchardt <[email protected]> wrote: > > On 8/27/26 10:21, Alexey Charkov wrote: > > Hello Heinrich, Tom, > > > > On Tue, Aug 25, 2026 at 11:45 PM Tom Rini <[email protected]> wrote: > >> > >> On Tue, Aug 25, 2026 at 09:37:43PM +0200, Heinrich Schuchardt wrote: > >>> On 7/31/26 14:40, Alexey Charkov wrote: > >>>> part_test_mac(), part_print_mac() and part_get_info_mac() each declare a > >>>> single-block buffer sized after the descriptor struct: > >>>> > >>>> ALLOC_CACHE_ALIGN_BUFFER(mac_driver_desc_t, ddesc, 1); > >>>> ALLOC_CACHE_ALIGN_BUFFER(mac_partition_t, mpart, 1); > >>>> > >>>> Both structs are 512 bytes, but every blk_dread() below them asks for one > >>>> block, which transfers desc->blksz bytes. On a device with 4096-byte > >>>> logical blocks that writes 4096 bytes into a 512-byte on-stack buffer and > >>>> corrupts the stack. > >>>> > >>>> part_test_mac() runs on every block device during partition probing, so on > >>>> sandbox with CONFIG_MAC_PARTITION=y this crashes on any access at all to a > >>>> device with large blocks, for instance: > >>>> > >>>> host bind 0 disk.img 4096 > >>>> part list host 0 > >>>> > >>>> Pad the buffers out to the block size with ALLOC_CACHE_ALIGN_BUFFER_PAD(), > >>>> which is what part_efi.c already does for its own block buffers. > >>>> > >>>> Signed-off-by: Alexey Charkov <[email protected]> > >>> > >>> Modern Apple Computers use GPT partition tables. > >>> > >>> Gemini claims: > >>> > >>> Because APM was functionally obsolete by the time Advanced Format (4Kn) > >>> drives emerged in the 2010s, Apple never updated the specification to > >>> establish official compatibility guidelines for native 4 KiB sectors. > >>> > >>> If Apple has not defined how to treat 4 KiB sectors, we should not try to > >>> invent a scheme. > >>> > >>> Apple has already deleted the documentation of this outdated partition table > >>> format (http://developer.apple.com/techpubs/mac/Devices/Devices-126.html) > >>> > >>> My preference would be to remove this driver completely. > >> > >> Yes, this is another case where we can just drop the driver and if > >> someone *needs* this for something, likely emulated, they can bring it > >> back. > > > > I've sent a separate patch [1] removing the driver altogether, which > > Heinrich has reviewed already. That one is against next though, and I > > believe these fixes would be great to have in main, so I'd rather keep > > this patch in the series for 2026.10. There will be a "change vs. > > remove" merge conflict when main is merged into next which will need > > to be resolved in favor of the removal. The upside is that tests don't > > keep crashing wherever a Mac partition table is probed for with a 4Kn > > storage device. > > > > Does that sound fine? > > Nobody seems to be using the MAC partition driver with 4 KiB devices. Sure, but the probe runs unconditionally whenever the driver is enabled, and it crashes then even if no MAC partition label is found. > I would rather drop this patch to make merging next after v2026.10 easier. > > Maybe Tom already wants to drop the MAC partition driver in 2026.10. That would work too - I just thought a deletion is not appropriate for the feature-freeze period. Best regards, Alexey