Discard improperly nested partitions.

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 ...

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.  */

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))

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;
+      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);
 
-      pos += sizeof (struct grub_partition_bsd_entry);
+      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;