Hi,
This is the reworked version of the patch.
There are few ramifications of this patch. First of all some partitions
which are just barely outside of the host partition will lead to
something like "partition not found" errors in grub-probe. This message
should be more informative (the easiest way is to issue a warning in
grub-probe if partitions are discarded except some cases where it's
known not to affect the functionality like 'd' "subpartitions", probably
such a warning in grub proper would be too annoying though).
There is now a grub_dprintf where the partition is discarded.
Then if you check partitions when iterating no need to recheck in
adjust_range.
I simplified the check in grub_disk_adjust_range: no need to check for
the ``ancestor'' partitions, but we still check for the given partition.
The patch still accepts sub-partitions that start at the same
(absolute) offset as the parent. For instance, in the above example,
ls -l in grub gives both (hd1,msdos1) and (hd1,msdos1,bsd1). Should
we discard (hd0,msdos1,bsd1), i.e. require that sub-partitions start
at a strictly positive relative offset?
I left this out. Even if it introduces redundancy, it's actually nice
to see (hd1,msdos1,bsd1) in ls -l (e.g., if bsd1 is the only partition
in the bsd label).
Grégoire
=== added file 'ChangeLog.strict-partition-nesting'
--- ChangeLog.strict-partition-nesting 1970-01-01 00:00:00 +0000
+++ ChangeLog.strict-partition-nesting 2010-07-06 21:25:28 +0000
@@ -0,0 +1,14 @@
+2010-07-06 Grégoire Sutre <gregoire.su...@gmail.com>
+
+ * kern/partition.c (grub_partition_validate): New function to check
+ that a partition is physically contained in its parent. Since offsets
+ are relative (and non-negative), this reduces to checking that the
+ partition ends before its parent.
+ (grub_partition_map_probe): Discard invalid partitions.
+ (grub_partition_iterate): Likewise.
+ * kern/disk.c (grub_disk_adjust_range): Simplify out-of-partition
+ check (partitions are assumed to be properly nested).
+ * 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.
=== modified file 'include/grub/partition.h'
--- include/grub/partition.h 2010-06-12 13:33:09 +0000
+++ include/grub/partition.h 2010-07-06 21:16:05 +0000
@@ -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. */
=== modified file 'kern/disk.c'
--- kern/disk.c 2010-02-07 00:48:38 +0000
+++ kern/disk.c 2010-07-06 21:22:27 +0000
@@ -361,12 +361,10 @@ grub_disk_adjust_range (grub_disk_t disk
*sector += *offset >> GRUB_DISK_SECTOR_BITS;
*offset &= GRUB_DISK_SECTOR_SIZE - 1;
- for (part = disk->partition; part; part = part->parent)
+ part = disk->partition;
+ if (part)
{
- grub_disk_addr_t start;
grub_uint64_t len;
-
- start = part->start;
len = part->len;
if (*sector >= len
@@ -374,7 +372,7 @@ grub_disk_adjust_range (grub_disk_t disk
>> GRUB_DISK_SECTOR_BITS))
return grub_error (GRUB_ERR_OUT_OF_RANGE, "out of partition");
- *sector += start;
+ *sector += grub_partition_get_start (part);
}
if (disk->total_sectors <= *sector
=== modified file 'kern/partition.c'
--- kern/partition.c 2010-02-06 20:00:53 +0000
+++ kern/partition.c 2010-07-06 21:20:04 +0000
@@ -23,6 +23,23 @@
grub_partition_map_t grub_partition_map_list;
+/*
+ * Checks that a partition is contained in its parent.
+ */
+static int
+grub_partition_validate (const grub_disk_t disk,
+ const grub_partition_t partition)
+{
+ grub_disk_addr_t parent_len;
+
+ if (disk->partition)
+ parent_len = grub_partition_get_len (disk->partition);
+ else
+ parent_len = disk->total_sectors;
+
+ return (partition->start + partition->len <= parent_len);
+}
+
static grub_partition_t
grub_partition_map_probe (const grub_partition_map_t partmap,
grub_disk_t disk, int partnum)
@@ -31,20 +48,26 @@ grub_partition_map_probe (const grub_par
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_validate (dsk, partition)))
+ {
+ grub_dprintf ("partition", "Invalid (sub-)partition %s%d (%s).\n",
+ partition->partmap->name, partition->number + 1,
+ "out of range");
+ 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 +161,15 @@ grub_partition_iterate (struct grub_disk
const grub_partition_t partition)
{
struct grub_partition p = *partition;
+
+ if (!(grub_partition_validate (dsk, partition)))
+ {
+ grub_dprintf ("partition", "Invalid (sub-)partition %s%d (%s).\n",
+ partition->partmap->name, partition->number + 1,
+ "out of range");
+ return 0;
+ }
+
p.parent = dsk->partition;
dsk->partition = 0;
if (hook (dsk, &p))
=== modified file 'partmap/bsdlabel.c'
--- partmap/bsdlabel.c 2010-03-26 14:44:13 +0000
+++ partmap/bsdlabel.c 2010-07-06 21:15:19 +0000
@@ -54,7 +54,7 @@ bsdlabel_partition_map_iterate (grub_dis
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,29 @@ bsdlabel_partition_map_iterate (grub_dis
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);
+ if (p.start < delta)
+ {
+ 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);
+ continue;
+ }
+ p.start -= delta;
p.len = grub_le_to_cpu32 (be.size);
p.partmap = &grub_bsdlabel_partition_map;
+ 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)
if (hook (disk, &p))
return grub_errno;
-
- pos += sizeof (struct grub_partition_bsd_entry);
}
return GRUB_ERR_NONE;
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
