Bug 197881

Summary: [PATCH] boot1.efi UEFI system table corruption
Product: Base System Reporter: Chris Ruffin <cmruffin>
Component: kernAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed FIXED    
Severity: Affects Some People CC: ambrisko, cmruffin, emaste
Priority: --- Keywords: uefi
Version: 10.1-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch which resolves disagreement on block size none

Description Chris Ruffin 2015-02-21 16:51:33 UTC
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.
Comment 1 Chris Ruffin 2015-02-23 15:09:29 UTC
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.
Comment 2 Ed Maste freebsd_committer freebsd_triage 2015-02-23 15:54:22 UTC
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.
Comment 3 Chris Ruffin 2015-02-23 17:44:38 UTC
I reviewed this change and agree it is equivalent / more complete fix.
Comment 4 commit-hook freebsd_committer freebsd_triage 2015-02-24 22:11:50 UTC
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
Comment 5 Ed Maste freebsd_committer freebsd_triage 2015-02-24 22:12:28 UTC
Resolved with partial merge of r273865