grub-devel
[Top][All Lists]
Advanced

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

Re: calculation overflow in grub_mm_init_region (patch)


From: Leif Lindholm
Subject: Re: calculation overflow in grub_mm_init_region (patch)
Date: Tue, 10 Sep 2013 14:13:34 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Thu, Aug 29, 2013 at 07:26:03PM +0200, Leif Lindholm wrote:
> When allocating memory for the heap on ARMv7 UEFI, the init code
> pretty much just allocates a chunk from the top of available RAM.
> 
> This means that when grub_mm_init_region is called for a region
> extending to the top of the 32-bit address space, addr + size == 0.
> However, this is not taken into account by arithmetic in this
> function, causing Grub to fail on one of my platforms[1] before
> "Welcome to GRUB".
> 
> Having some trouble getting my head around the grub_mm_init_region
> code right now (where is grub_mm_base initialised?), so don't have
> a patch.

So, sat down to dissect the code a bit, and turns out that apart from
a debug message (which is #ifdef 0 by default), grub_mm_init_region()
is actually innocent.

The error lies in get_header_from_pointer() (also kern/mm.c), and its
comparisons regarding whether a pointer lies within the current
region or not.

My suggested fix is as follows (with a little bit of refactoring to
help my brain).

/
    Leif

=== modified file 'grub-core/kern/mm.c'
--- grub-core/kern/mm.c 2013-04-20 15:39:49 +0000
+++ grub-core/kern/mm.c 2013-09-10 11:13:36 +0000
@@ -86,13 +86,20 @@
 static void
 get_header_from_pointer (void *ptr, grub_mm_header_t *p, grub_mm_region_t *r)
 {
+  grub_addr_t block_start = (grub_addr_t) ptr;
+
   if ((grub_addr_t) ptr & (GRUB_MM_ALIGN - 1))
     grub_fatal ("unaligned pointer %p", ptr);
 
   for (*r = grub_mm_base; *r; *r = (*r)->next)
-    if ((grub_addr_t) ptr > (grub_addr_t) ((*r) + 1)
-       && (grub_addr_t) ptr <= (grub_addr_t) ((*r) + 1) + (*r)->size)
-      break;
+    {
+      grub_addr_t region_start = (grub_addr_t) ((*r) + 1);
+      grub_addr_t region_end = (grub_addr_t) ((*r) + 1) + (*r)->size;
+
+      if (block_start > region_start)
+       if ((block_start <= region_end) || (region_end == 0))
+         break;
+    }
 
   if (! *r)
     grub_fatal ("out of range pointer %p", ptr);




reply via email to

[Prev in Thread] Current Thread [Next in Thread]