From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1LstC1-0007cl-9l for mharc-grub-devel@gnu.org; Sun, 12 Apr 2009 02:29:25 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LstBz-0007cU-7C for grub-devel@gnu.org; Sun, 12 Apr 2009 02:29:23 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LstBu-0007cF-FX for grub-devel@gnu.org; Sun, 12 Apr 2009 02:29:22 -0400 Received: from [199.232.76.173] (port=57230 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LstBu-0007cC-8c for grub-devel@gnu.org; Sun, 12 Apr 2009 02:29:18 -0400 Received: from c60.cesmail.net ([216.154.195.49]:40685) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.60) (envelope-from ) id 1LstBt-0001PN-Ty for grub-devel@gnu.org; Sun, 12 Apr 2009 02:29:18 -0400 Received: from unknown (HELO smtprelay2.cesmail.net) ([192.168.1.112]) by c60.cesmail.net with ESMTP; 12 Apr 2009 02:29:17 -0400 Received: from [192.168.0.22] (static-72-92-88-10.phlapa.fios.verizon.net [72.92.88.10]) by smtprelay2.cesmail.net (Postfix) with ESMTPSA id 6386334C6A for ; Sun, 12 Apr 2009 02:30:43 -0400 (EDT) From: Pavel Roskin To: The development of GRUB 2 In-Reply-To: <20090411.010833.154152360.davem@davemloft.net> References: <20090411.010833.154152360.davem@davemloft.net> Content-Type: text/plain Date: Sun, 12 Apr 2009 02:29:15 -0400 Message-Id: <1239517755.3887.24.camel@mj> Mime-Version: 1.0 X-Mailer: Evolution 2.24.5 (2.24.5-1.fc10) Content-Transfer-Encoding: 7bit X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. Subject: Re: [PATCH]: grub: Fix ofdisk disk cache corruption. 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: Sun, 12 Apr 2009 06:29:23 -0000 On Sat, 2009-04-11 at 01:08 -0700, David Miller wrote: > The ieee1275 ofdisk driver doesn't use a unique value for > disk->id so it's really easy to get disk corruption. I was > able to see such corruption by simply booting grub from one > disk and booting a Linux kernel from another, both of which > were on the same disk controller. I hope you mean disk cache corruption, as in the subject, not disk corruption. GRUB only writes to disks to save environment variables, and it's done very carefully. > +#define OFDISK_HASH_SZ 8 > +static struct ofdisk_hash_ent *ofdisk_hash[OFDISK_HASH_SZ]; > + > +static int > +ofdisk_hash_fn (const char *devpath) > +{ > + int hash = 0; > + while (*devpath) > + hash ^= *devpath++; > + return (hash & (OFDISK_HASH_SZ - 1)); > +} That's a 3 bit hash. The risk of collisions is very high. I would understand if you had 8 entries for the hash values, but the hash values themselves should be reasonably unique. > +static struct ofdisk_hash_ent * > +ofdisk_hash_add (char *devpath) > +{ > + struct ofdisk_hash_ent **head = &ofdisk_hash[ofdisk_hash_fn(devpath)]; > + struct ofdisk_hash_ent *p = grub_malloc(sizeof (*p)); > + > + if (p) > + { > + p->devpath = devpath; If you can save the device names, then there is no point in using hashes. You can use (long)devpath. > + if (!op) > + op = ofdisk_hash_add (devpath); > > - grub_ieee1275_open (devpath, &dev_ihandle); > + grub_free (devpath); But if you free the device names, then they are bad IDs. The probability of the same memory being reused for another name is high. Perhaps I misunderstand something. -- Regards, Pavel Roskin