Re: [edk2] [rdk2] 64-bit OVFM MemoryType bug [confirmed]

Subject: Re: [edk2] [rdk2] 64-bit OVFM MemoryType bug [confirmed]

From: Brendan Trotter <btrotter@gmail.com>

To: edk2-devel@lists.sourceforge.net

Date: 2011-05-13 05:54:37

Hi,

On Wed, May 11, 2011 at 9:58 PM, Brendan Trotter  wrote:
> I've been using OVFM (specifically "OVMF-X64-r11337-alpha.zip" and
> "OVMF-IA32-r11337-alpha.zip" from
> http://sourceforge.net/apps/mediawiki/tianocore/index.php?title=OVMF )
> running inside Qemu.
>
> I'm having issues with "AllocPages()" with "MemoryType = 0x80000000"
> on 64-bit OVFM. The "AllocPages()" call succeeds, but it causes
> "GetMemoryMap()" to crash later. If I use a normal memory type (e.g.
> "0x00000002 (loader data)" ) with no other changes at all, everything
> works correctly on 64-bit.
>
> I'm wondering if anyone can verify if this is a bug in OVFM (or some
> obscure problem in my code); and if it is a bug in OVFM whether
> someone wouldn't mind fixing and releasing a patched version, as it's
> the only 64-bit EFI environment I have available at the moment.

I've spent the last day debugging this with GDB.

The memory map is stored in what looks like a doubly-linked list of
structures. Allocating a page works fine (even for "MemoryType =
0x80000000") - I manually checked the previous/next links, etc. The
bug is in the "GetMemoryMap()" code (in EFI boot services).

"GetMemoryMap()" traverses the linked list of entries while copying
them to caller's buffer. When it gets to an entry with "MemoryType =
0x80000000" the entry is copied to the caller's buffer correctly, but
after copying each entry it does (what looks like) some strange lookup
with the MemoryType as the index in an array, causing a page fault
because the index is past the end of the array.

A dissassembly of the bug looks like this:

 0x0000000007fbbf0f:  mov    -0x40(%rbp),%rax   ;rax = address of source data
 0x0000000007fbbf13:  mov    0x38(%rax),%rdx    ;rdx = 0x0000000f = attributes
 0x0000000007fbbf17:  mov    0x18(%rbp),%rax    ;rax = address in dest buffer
 0x0000000007fbbf1b:  mov    %rdx,0x20(%rax)    ;Store attributes in dest buffer
 0x0000000007fbbf1f:  mov    0x18(%rbp),%rax    ;rax = address in dest
buffer (again)
 0x0000000007fbbf23:  mov    (%rax),%eax        ;rax = MemoryType = 0x80000000
 0x0000000007fbbf25:  movabs $0x7fcb0c0,%rcx    ;rcx = something (base
address of an array?)
 0x0000000007fbbf2f:  mov    %eax,%edx          ;rdx = MemoryType = 0x80000000
 0x0000000007fbbf31:  mov    %rdx,%rax          ;rax = MemoryType = 0x80000000
 0x0000000007fbbf34:  add    %rax,%rax          ;rax = MemoryType * 2
= 0x0000000100000000
 0x0000000007fbbf37:  add    %rdx,%rax          ;rax = MemoryType * 3
= 0x0000000180000000
 0x0000000007fbbf3a:  shl    $0x4,%rax          ;rax = MemoryType * 48
= 0x0000001800000000
 0x0000000007fbbf3e:  add    %rcx,%rax          ;rax = base address of
array + MemoryType * 48 = 0x0000001807FCB0C0
 0x0000000007fbbf41:  add    $0x20,%rax         ;rax = base address of
array + MemoryType * 48 + 32 = 0x0000001807FCB0E0
 0x0000000007fbbf45:  movzbl 0x9(%rax),%eax     ;Page fault ("array
index out of bounds")

 0x0000000007fbbf49:  test   %al,%al            ;Test whatever was
read from the array
 0x0000000007fbbf4b:  je     0x7fbbf6a          ;If whatever was read
from the array is zero...
                                                ;"Dodgy" value of RAX
not used after this (regardless of whether branch taken or not taken)

Converting this back in to "C-like" code, it would probably look something like:

dest->attributes = src->attributes;
if( some_array[ memory_type] != 0) {
    dest->attributes |= EFI_MEMORY_RUNTIME;
}

The maximum value for "MemoryType" is 0xFFFFFFFF and each entry in the
array is 48 bytes (on 64-bit 80x86 at least), so this array would need
to consume about 192 GiB of RAM. Obviously it's a bug.

Please remember that while these values for MemoryType are less
common, they are perfectly valid for an OS loader. The EFI
specification (all versions from 1.1 to 2.3), in the description of
the "AllocPages()" function's parameters, say: "MemoryType values in
the range 0x80000000..0xFFFFFFFF are reserved for use by UEFI OS
loaders that are provided by operating system vendors. The only
illegal memory type values are those in the range
EfiMaxMemoryType..0x7FFFFFFF."

I assume the same code is used on 32-bit systems, but because 32-bit
systems don't use paging there is no page fault. The code that
calculates the address of the entry in the array would overflow and
read a byte from a "random" address, and if you're lucky it works
without any symptoms of the bug (just with the potentially wrong value
for the "EFI_MEMORY_RUNTIME" flag in the entry's attributes).

Also, I've tested this in both Qemu and VirtualBox, and it's the same
bug in both cases. VirtualBox uses an older version of OVFM but I'm
not too sure how old - when entering the shell it says "EFI Shell
version 2.10 [1.1]" while the version I'm using in Qemu
("OVMF-X64-r11337-alpha.zip") says "EFI Shell version 2.30 [1.1]".

Hopefully this will help....


Cheers,

Brendan

------------------------------------------------------------------------------
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel