Robert Millan wrote:
On Tue, Oct 23, 2007 at 09:06:16PM +0200, Christian Franke wrote:
+/* Check memory address */
+static int
+addr_is_valid (grub_addr_t addr)
+{
+ volatile unsigned char * p = (volatile unsigned char *)addr;
+ unsigned char x, y;
+ x = *p;
+ *p = x ^ 0xcf;
+ y = *p;
+ *p = x;
+ return y == (x ^ 0xcf);
+}
0xff would be better IMO.
Done.
+ if (!(addr + size > addr && addr_is_valid (addr) && addr_is_valid
(addr+size-1)))
+ grub_fatal ("invalid memory region %p - %p", (char*)addr,
(char*)addr+size-1);
Should `addr + size > addr' be optimized out as `size > 0' ? (or if we need it
this way to check for overflows, should we prevent gcc from optimizing it?)
Good point.
It worked (I usually test all corner cases before patch release). And a
review of .S file shows that gcc (3.4.4) does not optimize here. I'm not
sure whether that would be allowed.
But you are right - such check should not depend on specific overflow
behaviour. I've changed this.
New patch attached, old changelog still valid.
Christian
--- grub2.orig/kern/i386/pc/init.c 2007-10-22 22:22:51.359375000 +0200
+++ grub2/kern/i386/pc/init.c 2007-11-19 22:11:19.796875000 +0100
@@ -83,6 +83,19 @@ make_install_device (void)
return grub_prefix;
}
+/* Check memory address. */
+static int
+addr_is_valid (grub_addr_t addr)
+{
+ volatile unsigned char * p = (volatile unsigned char *)addr;
+ unsigned char x = *p;
+ unsigned char y = ~x;
+ *p = y;
+ unsigned char t = *p;
+ *p = x;
+ return y == t;
+}
+
/* Add a memory region. */
static void
add_mem_region (grub_addr_t addr, grub_size_t size)
@@ -91,6 +104,10 @@ add_mem_region (grub_addr_t addr, grub_s
/* Ignore. */
return;
+ if (!(0 < size && size - 1 <= ~(grub_addr_t)0 - addr &&
+ addr_is_valid (addr) && addr_is_valid (addr + size - 1)))
+ grub_fatal ("invalid memory region %p - %p", (char*)addr, (char*)addr + size - 1);
+
mem_regions[num_regions].addr = addr;
mem_regions[num_regions].size = size;
num_regions++;
@@ -199,13 +216,8 @@ grub_machine_init (void)
if (eisa_mmap)
{
- if ((eisa_mmap & 0xFFFF) == 0x3C00)
- add_mem_region (0x100000, (eisa_mmap << 16) + 0x100000 * 15);
- else
- {
- add_mem_region (0x100000, (eisa_mmap & 0xFFFF) << 10);
- add_mem_region (0x1000000, eisa_mmap << 16);
- }
+ add_mem_region (0x100000, (eisa_mmap & 0xFFFF) << 10);
+ add_mem_region (0x1000000, eisa_mmap & ~0xFFFF);
}
else
add_mem_region (0x100000, grub_get_memsize (1) << 10);
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
