From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1IuEMQ-00056e-5C for mharc-grub-devel@gnu.org; Mon, 19 Nov 2007 16:40:54 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IuEMN-00056C-Px for grub-devel@gnu.org; Mon, 19 Nov 2007 16:40:51 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IuEML-000533-CZ for grub-devel@gnu.org; Mon, 19 Nov 2007 16:40:50 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IuEML-00052u-50 for grub-devel@gnu.org; Mon, 19 Nov 2007 16:40:49 -0500 Received: from mailout02.sul.t-online.de ([194.25.134.17] helo=mailout02.sul.t-online.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1IuEMK-00053J-Lx for grub-devel@gnu.org; Mon, 19 Nov 2007 16:40:48 -0500 Received: from fwd26.aul.t-online.de by mailout02.sul.t-online.com with smtp id 1IuEMJ-0004aT-03; Mon, 19 Nov 2007 22:40:47 +0100 Received: from [10.3.2.2] (VgunukZAQhBtEwwbDolzw2OAPmgSRMKm+YC1ELb1-4Qff+OwCMWDNeonndiPgmNZ9+@[217.235.205.150]) by fwd26.aul.t-online.de with esmtp id 1IuEME-1bDu9g0; Mon, 19 Nov 2007 22:40:42 +0100 Message-ID: <474202DA.4010001@t-online.de> Date: Mon, 19 Nov 2007 22:40:42 +0100 From: Christian Franke User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070802 SeaMonkey/1.1.4 MIME-Version: 1.0 To: The development of GRUB 2 References: <471E4628.9030706@t-online.de> <20071118112737.GB16674@thorin> In-Reply-To: <20071118112737.GB16674@thorin> Content-Type: multipart/mixed; boundary="------------020602000202020909020009" X-ID: VgunukZAQhBtEwwbDolzw2OAPmgSRMKm+YC1ELb1-4Qff+OwCMWDNeonndiPgmNZ9+ X-TOI-MSGID: b6763c78-2f1b-40a1-abfe-5990cd10fd8b X-detected-kernel: by monty-python.gnu.org: Linux 2.6 (newer, 3) Subject: Re: [PATCH] Fix eisa_mmap evaluation, add memory existence check X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Nov 2007 21:40:52 -0000 This is a multi-part message in MIME format. --------------020602000202020909020009 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 --------------020602000202020909020009 Content-Type: text/x-patch; name="grub2-init-eisa_mmap-2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="grub2-init-eisa_mmap-2.patch" --- 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); --------------020602000202020909020009--