[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] Make XEN_FW_EFI_MEM_INFO easier to use



The XEN_FW_EFI_MEM_INFO platform op has very surprising behavior: it
only sets info->mem.size if the initial value was *larger* than the size
of the memory region.  This is not particularly useful and cost me most
of a day of debugging.  It also has some integer overflow problems,
though as the data comes from dom0 or the firmware (both of which are
trusted) these are not security issues.

Fix both of these problems by unconditionally setting the memory region
size and by computing it in a way that is immune to integer overflow.
The new code is slightly longer, but it is much easier to understand and
use.
---
 xen/common/efi/runtime.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 
a8fc2b99ae098d74af1978bdf58212eb99cce70f..a086850c9b0bbb6e4dd3ccca647c09d346f87c55
 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -269,19 +269,21 @@
     case XEN_FW_EFI_MEM_INFO:
         for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
         {
+            uint64_t len;
             EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
-            u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
+
+            if ( desc->NumberOfPages > (UINT64_MAX >> EFI_PAGE_SHIFT) )
+                len = UINT64_MAX;
+            else
+                len = desc->NumberOfPages << EFI_PAGE_SHIFT;
 
             if ( info->mem.addr >= desc->PhysicalStart &&
-                 info->mem.addr < desc->PhysicalStart + len )
+                 info->mem.addr - desc->PhysicalStart < len )
             {
                 info->mem.type = desc->Type;
                 info->mem.attr = desc->Attribute;
-                if ( info->mem.addr + info->mem.size < info->mem.addr ||
-                     info->mem.addr + info->mem.size >
-                     desc->PhysicalStart + len )
-                    info->mem.size = desc->PhysicalStart + len -
-                                     info->mem.addr;
+                info->mem.size = len - (info->mem.addr - desc->PhysicalStart);
+
                 return 0;
             }
         }
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.