[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);
- Re: calculation overflow in grub_mm_init_region (patch),
Leif Lindholm <=