grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH]: grub: Fix ofdisk disk cache corruption.


From: David Miller
Subject: [PATCH]: grub: Fix ofdisk disk cache corruption.
Date: Sat, 11 Apr 2009 01:08:33 -0700 (PDT)

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.

The solution implemented here is to create a hash table of
OF device paths openned, and use the address of the hash
table entry as disk->id

2009-04-11  David S. Miller  <address@hidden>

        * disk/ieee1275/ofdisk.c (struct ofdisk_hash_ent): New struct.
        (OFDISK_HASH_SZ): Define.
        (ofdisk_hash): New hash table.
        (ofdisk_hash_fn, ofdisk_hash_find, ofdisk_hash_add): New functions.
        (grub_ofdisk_open): Use ofdisk_hash_ent address as disk->id
        instead of device phandle which is not unique.
---
 disk/ieee1275/ofdisk.c |   70 +++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/disk/ieee1275/ofdisk.c b/disk/ieee1275/ofdisk.c
index fec56be..a56433d 100644
--- a/disk/ieee1275/ofdisk.c
+++ b/disk/ieee1275/ofdisk.c
@@ -23,6 +23,53 @@
 #include <grub/ieee1275/ieee1275.h>
 #include <grub/ieee1275/ofdisk.h>
 
+struct ofdisk_hash_ent
+{
+  char *devpath;
+  struct ofdisk_hash_ent *next;
+};
+
+#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));
+}
+
+static struct ofdisk_hash_ent *
+ofdisk_hash_find (const char *devpath)
+{
+  struct ofdisk_hash_ent *p = ofdisk_hash[ofdisk_hash_fn(devpath)];
+
+  while (p)
+    {
+      if (!grub_strcmp (p->devpath, devpath))
+       break;
+      p = p->next;
+    }
+  return p;
+}
+
+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;
+      p->next = *head;
+      *head = p;
+    }
+  return p;
+}
+
 static int
 grub_ofdisk_iterate (int (*hook) (const char *name))
 {
@@ -76,6 +123,7 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
 {
   grub_ieee1275_phandle_t dev;
   grub_ieee1275_ihandle_t dev_ihandle = 0;
+  struct ofdisk_hash_ent *op;
   char *devpath;
   /* XXX: This should be large enough for any possible case.  */
   char prop[64];
@@ -89,18 +137,26 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
   if (! grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_NO_PARTITION_0))
     grub_strcat (devpath, ":0");
 
-  grub_dprintf ("disk", "Opening `%s'.\n", devpath);
+  op = ofdisk_hash_find (devpath);
+  if (!op)
+    op = ofdisk_hash_add (devpath);
 
-  grub_ieee1275_open (devpath, &dev_ihandle);
+  grub_free (devpath);
+  if (!op)
+    return grub_errno;
+
+  grub_dprintf ("disk", "Opening `%s'.\n", op->devpath);
+
+  grub_ieee1275_open (op->devpath, &dev_ihandle);
   if (! dev_ihandle)
     {
       grub_error (GRUB_ERR_UNKNOWN_DEVICE, "Can't open device");
       goto fail;
     }
 
-  grub_dprintf ("disk", "Opened `%s' as handle %p.\n", devpath, (void *) 
dev_ihandle);
+  grub_dprintf ("disk", "Opened `%s' as handle %p.\n", op->devpath, (void *) 
dev_ihandle);
 
-  if (grub_ieee1275_finddevice (devpath, &dev))
+  if (grub_ieee1275_finddevice (op->devpath, &dev))
     {
       grub_error (GRUB_ERR_UNKNOWN_DEVICE, "Can't read device properties");
       goto fail;
@@ -124,20 +180,16 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
      is possible to use seek for this.  */
   disk->total_sectors = 0xFFFFFFFFUL;
 
-  /* XXX: Is it ok to use this?  Perhaps it is better to use the path
-     or some property.  */
-  disk->id = dev;
+  disk->id = (unsigned long) op;
 
   /* XXX: Read this, somehow.  */
   disk->has_partitions = 1;
   disk->data = (void *) dev_ihandle;
-  grub_free (devpath);
   return 0;
 
  fail:
   if (dev_ihandle)
     grub_ieee1275_close (dev_ihandle);
-  grub_free (devpath);
   return grub_errno;
 }
 
-- 
1.6.2.2





reply via email to

[Prev in Thread] Current Thread [Next in Thread]