On Sun, 26 Jan 2020 07:57:04 +0000
Mohamed Akram <[email protected]> wrote:
> 16 partitions:
> # size offset fstype [fsize bsize cpg]
> a: 2097152 3802170304 4.2BSD 2048 16384 12958
> ...
>
> A fellow OpenBSD user on this list suggested that this is due to a bug
> in OpenBSD's BOOTX64.EFI - that it only reads the first 1024GB (2^31
> sectors) of the disk while my partition is at sector 3802170304 causing
> a daddr32_t overflow in /sys/arch/amd64/stand/efiboot/efidev.c which
> then returns EINVAL (invalid argument).
I'm the user who saw the report. I believe that efidev.c has limits:
- the disklabel must be before sector 4_294_967_296 (first 2048G).
- partition 'a' must be before sector 2_147_483_648 (first 1024G).
This disk has its disklabel and 'a' at sector offset 3_802_170_304,
so BOOTX64.EFI would read the disklabel, but fail to read the kernel.
efidev.c findopenbsd_gpt() would find the GPT partition and check
3_802_170_304 > UINT_MAX 4_294_967_295:
if (found) {
lba = letoh64(gp.gp_lba_start);
/* Bootloaders do not current handle addresses > UINT_MAX! */
if (lba > UINT_MAX || EFI_SECTOBLK(ed, lba) > UINT_MAX) {
*err = "OpenBSD Partition LBA > 2**32 - 1";
return (-1);
}
...
The check is false, so findopenbsd_gpt() would succeed. (EFI_SECTOBLK
converts from sectors to 512-byte blocks, but this disk has 512 bytes
per sector, so the converted offset is the same.)
Reading partition 'a' would call efidev.c efistrategy():
int
efistrategy(void *devdata, int rw, daddr32_t blk, size_t size, void *buf,
size_t *rsize)
{
...
blk += DL_SECTOBLK(&dip->disklabel,
dip->disklabel.d_partitions[B_PARTITION(dip->bsddev)].p_offset);
if (blk < 0)
error = EINVAL;
else
error = dip->diskio(rw, dip, blk, nsect, buf);
This looks like an integer overflow, because p_offset would be
3_802_170_304, but daddr32_t is signed (max 2_147_483_647). Then
blk < 0 would be true, and EINVAL "Invalid argument" would appear.
All my disks are smaller than 1024G, so I would never encounter this
problem. Other people might have larger disks, but if they give the
whole disk to OpenBSD, then partition 'a' would go near the beginning
of the disk, so they would not have this problem.
I suggest to change the prototypes of diskio and strategy in
/sys/arch/amd64/stand/libsa/disk.h struct diskinfo to use 64-bit
offsets, but this might break a bunch of bootloaders.
--George
