From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZtwMI-0006ch-Ol for mharc-grub-devel@gnu.org; Wed, 04 Nov 2015 06:36:06 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36733) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtwMG-0006cb-Ry for grub-devel@gnu.org; Wed, 04 Nov 2015 06:36:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtwMD-00063P-L6 for grub-devel@gnu.org; Wed, 04 Nov 2015 06:36:04 -0500 Received: from mail-lb0-x22d.google.com ([2a00:1450:4010:c04::22d]:36345) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtwMD-00063L-Ei for grub-devel@gnu.org; Wed, 04 Nov 2015 06:36:01 -0500 Received: by lbjm5 with SMTP id m5so14205886lbj.3 for ; Wed, 04 Nov 2015 03:36:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-type:content-transfer-encoding; bh=7frwEDWVxLtclC38hcw3dODZC2JI8na0ohxS7NwA1Fg=; b=F3g3l+j1NXq8fBexSQFPdwVAnYEOGInndCs4Jk3OAPzYpXe1ReZCvvaLIDS4ZXrDsa txrXr6Pxh7s88jZC8ziDR9H9P9Cfu5u37fwxV4ddz+ulIEgEDCHet5pEPXMTZ4ulAI7N +zfdq3Fk+n00PemXa0qQbg4uT9hGIjY1Rs0FylgIJVIXdbeDHycAZFobnYWuebp6WMhG kiUUQRHAit4P237SzDuLHfBFpslopFfO425oFqxTFhnRqtWAFJQTRI1SC6tis6ZIcbWg nTkzJwHlntJcTAQWTRjB3T4rfsrwr+K5zq33ukYZ4qwt5pu9AAUE8BcEDHFyiCGPTKW3 4U1A== X-Received: by 10.112.181.164 with SMTP id dx4mr723224lbc.29.1446636960802; Wed, 04 Nov 2015 03:36:00 -0800 (PST) Received: from [192.168.1.41] (ppp91-76-25-247.pppoe.mtu-net.ru. [91.76.25.247]) by smtp.gmail.com with ESMTPSA id dz9sm138399lbc.40.2015.11.04.03.35.59 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 04 Nov 2015 03:36:00 -0800 (PST) Subject: Re: [coreboot] Dell Dimension 8300 reboots when grub2 cbfs module is loaded To: Nico Huber , The development of GNU GRUB , coreboot@coreboot.org References: <56362757.3070801@gmail.com> <563929F3.7050809@gmx.de> From: Andrei Borzenkov Message-ID: <5639ED9F.5070606@gmail.com> Date: Wed, 4 Nov 2015 14:35:59 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <563929F3.7050809@gmx.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4010:c04::22d X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Nov 2015 11:36:06 -0000 04.11.2015 00:41, Nico Huber пишет: > Hi Andrei, > > your patch looks good generally, but the check is off by one. It's Yes, but it should not matter in real life. Last 4 bytes are pointer, so header cannot start after 0xffffffdc anyway. I'll change it. > obvious, we want to have sane checking there. Reading from a random > address can cause trouble and 0xffffffff is not the only offending > address. > > On x86, the cbfs is mapped right below the 4GiB line. Current machines > don't have more than 16MiB space for cbfs, FWIW. So maybe it's best to > check if the ptr points somewhere between 0xff000000 and (0x100000000 - > sizeof(*head)). > That's far exceeds my intention and knowledge, sorry. > Nico > > On 01.11.2015 15:53, Andrei Borzenkov wrote: >> I was debugging problem reported by user on Dell Dimension 8300 - it >> rebooted when doing "ls -l". It turned out, the problem was triggered by >> loading cbfs which probed for header. System has 2GB memory, and attempt >> to read from address 0xffffffff caused instant reboot. 0xffffffff was >> returned by read from non-existing address 0xfffffffc. >> >> The proof of concept patch below avoids it, but I wonder what the proper >> fix is. >> >> diff --git a/grub-core/fs/cbfs.c b/grub-core/fs/cbfs.c >> index a34eb88..a5a2fde 100644 >> --- a/grub-core/fs/cbfs.c >> +++ b/grub-core/fs/cbfs.c >> @@ -344,8 +344,9 @@ init_cbfsdisk (void) >> >> ptr = *(grub_uint32_t *) 0xfffffffc; >> head = (struct cbfs_header *) (grub_addr_t) ptr; >> + grub_dprintf ("cbfs", "head=%p\n", head); >> >> - if (!validate_head (head)) >> + if (0xffffffff - ptr < sizeof (*head) || !validate_head (head)) >> return; >> >> cbfsdisk_size = ALIGN_UP (grub_be_to_cpu32 (head->romsize), >> >>