boot1.efi from 10.1-RELEASE can cause data corruption of its own UEFI system table pointer in memory. The EfiBlockIoProtocol instance associated with a CDROM parition typically has a block size of 2048. boot1.efi assumes a block size of 512 in the call to dskread() to read the superblock, yet the dskread() function in boot1.efi uses the block size from the BlockIoProtocol instance. This causes the call to BlkIoProtocol->ReadBlocks() in dskread() to expose a larger-than-actual buffer size to UEFI FW, which will cause the ReadBlocks() call to overrun the dmadat buffer. So if a partition is present in the system with a block size of 2048 (such as cdrom media partitions), boot1.efi can overrun its dmadat structure and cause corruption of other global data including a global pointe r to the UEFI system table. Failure sequence: 1) dmadat.sbbuf is allocated 8192 bytes at build time (SBLOCKSIZE) 2) Boot1.efi retrieves handles with all instances of BlockIoProtocol. 3) If cdrom partition is present in the system, UEFI FW will return a BlockIoProtocol of the CDROM partition which has a BlockIoProtocol->Media->BlockSize = 2048. 4) domount() is called for this BlockIoProtocol instance. 5) fsread() is called to read the superblock 6) Boot1.efi dskread() is called with nblk=16 (fixed SBLOCKSIZE (8192) / fixed DEV_BSIZE (512)). This is assuming a block size of 512. 7) dskread() in boot1.efi calls status = bootdev->ReadBlocks(bootdev, bootdev->Media->MediaId, lba, nblk * bootdev->Media->BlockSize, buf); Here nblk=16 but bootdev->Media->BlockSize is 2048 which results in passing a Buffer Size argument of 16 * 2048 = 32768 which overruns the dmadat->sbbuf fixed allocation. 8) UEFI ReaadBlocks() overruns dmadat and overwrites the global pointer to the UEFI system table (sets it to zero) 9) A subsequent call to systab->BootServices->HandleProtocol() causes path of exeution at physical address 0x98 which does not contain valid code. 10) Processor eventually raises an invalid opcode exception.
Created attachment 153375 [details] patch which resolves disagreement on block size ufsread.c assumes a device block size of 512 in all calls to dskread() which is implemented in boot1.c. This patch causes dskread() to follow that same assumption.
Looks like this was fixed in a slightly different way on HEAD, to support 4Kn drives, here: https://svnweb.freebsd.org/base?view=revision&revision=273865 (or git 37cb22f7) That change also divides lba by bootdev->Media->BlockSize / DEV_BSIZE - i.e., by 4 for CDs and by 8 for 4Kn.
I reviewed this change and agree it is equivalent / more complete fix.
A commit references this bug: Author: emaste Date: Tue Feb 24 22:11:08 UTC 2015 New revision: 279254 URL: https://svnweb.freebsd.org/changeset/base/279254 Log: MFC part of r273865: fix boot1.efi for block size != 512 r273865 is part of the work for supporting 4Kn drives, but it turns out the underlying bug can actually cause corruption of the UEFI system table in any case where block size is not 512. Relevant portion of the original commit message: convert boot1.efi to corrrectly calculate the lba for what the media reports and convert the size based on what FreeBSD uses. existing code would use the 512 byte lba and convert the using 4K byte size. PR: 197881 Reviewed by: Chris Ruffin Changes: stable/10/sys/boot/amd64/boot1.efi/boot1.c
Resolved with partial merge of r273865