Discard improperly nested partitions.

diff --git a/ChangeLog b/ChangeLog
index e894a479c1f474bcb6d8ceb640cefb990230ce44..8511fa2cf7a0c27894f3736f8dd4360a943cd082 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2010-07-14  Grégoire Sutre  <gregoire.sutre@gmail.com>
+
+       * kern/partition.c (grub_partition_check_containment): New function to
+       check that a partition is physically contained in a parent.  Since
+       offsets are relative (and non-negative), this reduces to checking that
+       the partition ends before its parent.
+       (grub_partition_map_probe): Discard out-of-range sub-partitions.
+       (grub_partition_iterate): Likewise.
+       * include/grub/partition.h (grub_partition_map): Slightly more detailed
+       comments.
+       * partmap/bsdlabel.c (bsdlabel_partition_map_iterate): Discard
+       partitions that start before their parent, and add debug printfs.
+
 2010-07-13  Colin Watson  <cjwatson@ubuntu.com>
 
        * Makefile.in (.SUFFIX): Spell correctly, as ...

diff --git a/include/grub/partition.h b/include/grub/partition.h
index a23e94e0723aaa8819f6439782dc1ec826476519..20705c5276d0331508d33251111a7dd3492d7e96 100644 (file)
--- a/include/grub/partition.h
+++ b/include/grub/partition.h
@@ -48,7 +48,7 @@ struct grub_partition
   /* The partition number.  */
   int number;
 
-  /* The start sector.  */
+  /* The start sector (relative to parent).  */
   grub_disk_addr_t start;
 
   /* The length in sector units.  */
@@ -60,7 +60,7 @@ struct grub_partition
   /* The index of this partition in the partition table.  */
   int index;
 
-  /* Parent partition map.  */
+  /* Parent partition (physically contains this partition).  */
   struct grub_partition *parent;
 
   /* The type partition map.  */

diff --git a/kern/partition.c b/kern/partition.c
index 2a33ac32989660fba7982fec686f98b7e162084d..a1e37f675edad09be4dc51308d1b0e0eeada1df0 100644 (file)
--- a/kern/partition.c
+++ b/kern/partition.c
@@ -23,6 +23,37 @@
 
 grub_partition_map_t grub_partition_map_list;
 
+/*
+ * Checks that disk->partition contains part.  This function assumes that the
+ * start of part is relative to the start of disk->partition.  Returns 1 if
+ * disk->partition is null.
+ */
+static int
+grub_partition_check_containment (const grub_disk_t disk,
+                                 const grub_partition_t part)
+{
+  if (disk->partition == NULL)
+    return 1;
+
+  if (part->start + part->len > disk->partition->len)
+    {
+      char *partname;
+
+      partname = grub_partition_get_name (disk->partition);
+      grub_dprintf ("partition", "sub-partition %s%d of (%s,%s) ends after parent.\n",
+                   part->partmap->name, part->number + 1, disk->name, partname);
+#ifdef GRUB_UTIL
+      grub_util_warn ("Discarding improperly nested partition (%s,%s,%s%d)",
+                     disk->name, partname, part->partmap->name, part->number + 1);
+#endif
+      grub_free (partname);
+
+      return 0;
+    }
+
+  return 1;
+}
+
 static grub_partition_t
 grub_partition_map_probe (const grub_partition_map_t partmap,
                          grub_disk_t disk, int partnum)
@@ -31,20 +62,21 @@ grub_partition_map_probe (const grub_partition_map_t partmap,
 
   auto int find_func (grub_disk_t d, const grub_partition_t partition);
 
-  int find_func (grub_disk_t d __attribute__ ((unused)),
+  int find_func (grub_disk_t dsk,
                 const grub_partition_t partition)
     {
-      if (partnum == partition->number)
-       {
-         p = (grub_partition_t) grub_malloc (sizeof (*p));
-         if (! p)
-           return 1;
+      if (partnum != partition->number)
+       return 0;
 
-         grub_memcpy (p, partition, sizeof (*p));
-         return 1;
-       }
+      if (!(grub_partition_check_containment (dsk, partition)))
+       return 0;
 
-      return 0;
+      p = (grub_partition_t) grub_malloc (sizeof (*p));
+      if (! p)
+       return 1;
+
+      grub_memcpy (p, partition, sizeof (*p));
+      return 1;
     }
 
   partmap->iterate (disk, find_func);
@@ -138,6 +170,10 @@ grub_partition_iterate (struct grub_disk *disk,
                    const grub_partition_t partition)
     {
       struct grub_partition p = *partition;
+
+      if (!(grub_partition_check_containment (dsk, partition)))
+       return 0;
+
       p.parent = dsk->partition;
       dsk->partition = 0;
       if (hook (dsk, &p))

diff --git a/partmap/bsdlabel.c b/partmap/bsdlabel.c
index d28f36d073decc4a918252b421d54019986f7001..d38a0eaf3c9f056378fcf3f26cfada19bdbf73e7 100644 (file)
--- a/partmap/bsdlabel.c
+++ b/partmap/bsdlabel.c
@@ -54,7 +54,7 @@ bsdlabel_partition_map_iterate (grub_disk_t disk,
 
   for (p.number = 0;
        p.number < grub_cpu_to_le16 (label.num_partitions);
-       p.number++)
+       p.number++, pos += sizeof (struct grub_partition_bsd_entry))
     {
       struct grub_partition_bsd_entry be;
 
@@ -64,15 +64,43 @@ bsdlabel_partition_map_iterate (grub_disk_t disk,
       if (grub_disk_read (disk, p.offset, p.index, sizeof (be),  &be))
        return grub_errno;
 
-      p.start = grub_le_to_cpu32 (be.offset) - delta;
+      p.start = grub_le_to_cpu32 (be.offset);
       p.len = grub_le_to_cpu32 (be.size);
       p.partmap = &grub_bsdlabel_partition_map;
 
-      if (be.fs_type != GRUB_PC_PARTITION_BSD_TYPE_UNUSED)
-       if (hook (disk, &p))
-         return grub_errno;
-
-      pos += sizeof (struct grub_partition_bsd_entry);
+      grub_dprintf ("partition",
+                   "partition %d: type 0x%x, start 0x%llx, len 0x%llx\n",
+                   p.number, be.fs_type,
+                   (unsigned long long) p.start,
+                   (unsigned long long) p.len);
+
+      if (be.fs_type == GRUB_PC_PARTITION_BSD_TYPE_UNUSED)
+       continue;
+
+      if (p.start < delta)
+       {
+#ifdef GRUB_UTIL
+         char *partname;
+#endif
+         grub_dprintf ("partition",
+                       "partition %d: invalid start (found 0x%llx, wanted >= 0x%llx)\n",
+                       p.number,
+                       (unsigned long long) p.start,
+                       (unsigned long long) delta);
+#ifdef GRUB_UTIL
+         /* disk->partition != NULL as 0 < delta */
+         partname = grub_partition_get_name (disk->partition);
+         grub_util_warn ("Discarding improperly nested partition (%s,%s,%s%d)",
+                         disk->name, partname, p.partmap->name, p.number + 1);
+         grub_free (partname);
+#endif
+         continue;
+       }
+
+      p.start -= delta;
+
+      if (hook (disk, &p))
+       return grub_errno;
     }
 
   return GRUB_ERR_NONE;