Hi, Regarding the nested partition code, there is an implicit assumption that each partition should be contained in its parent, i.e. its sectors should also be sectors of its parent.
This ``physical nesting'' is checked in grub_disk_read, but it would be better to check it before that. The attached patch discards partitions that are invalid w.r.t. physical nesting. This solves, in particular, a problem related to NetBSD (and OpenBSD) disklabels. With this patch, ``external'' partitions in a disklabel simply do not show up as BSD partitions. For instance (see bug #29956 for an image): MBR Partition table: 0: NetBSD start 32, size 1000 1: DOS start 1040, size 1000 NetBSD Disklabel (stored in MBR partition 0) 5 partitions: # size offset fstype [fsize bsize cpg/sgs] a: 1000 32 4.2BSD c: 1000 32 unused d: 2048 0 unused e: 1000 1040 MSDOS The e: partition is external: it is not contained in MBR NetBSD partition. Without the patch, we get: $ grub-probe -m /dev/null -t drive -d /dev/rvnd0e (/dev/rvnd0d,1,5) # this is (/dev/rvnd0d,msdos1,bsd5) $ grub-probe -m /dev/null -t fs -d /dev/rvnd0e grub-probe: error: unknown filesystem. With the patch, we get: niagara# grub-probe -m /dev/null -t drive -d /dev/rvnd0e (/dev/rvnd0d,2) # this is (/dev/rvnd0d,msdos2) niagara# grub-probe -m /dev/null -t fs -d /dev/rvnd0e fat 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? Grégoire
--- grub_trunk/kern/partition.c 2010-05-28 01:50:37.000000000 +0200 +++ grub_trunk_new/kern/partition.c 2010-05-28 01:53:13.000000000 +0200 @@ -16,32 +16,50 @@ * along with GRUB. If not, see <http://www.gnu.org/licenses/>. */ #include <grub/misc.h> #include <grub/mm.h> #include <grub/partition.h> #include <grub/disk.h> 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) { grub_partition_t p = 0; 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) + if (partnum == partition->number && + grub_partition_validate (dsk, partition)) { p = (grub_partition_t) grub_malloc (sizeof (*p)); if (! p) return 1; grub_memcpy (p, partition, sizeof (*p)); return 1; } return 0; @@ -131,20 +149,24 @@ grub_partition_iterate (struct grub_disk const grub_partition_t partition)) { int ret = 0; auto int part_iterate (grub_disk_t dsk, const grub_partition_t p); int part_iterate (grub_disk_t dsk, const grub_partition_t partition) { struct grub_partition p = *partition; + + if (!(grub_partition_validate (dsk, partition))) + return 0; + p.parent = dsk->partition; dsk->partition = 0; if (hook (dsk, &p)) { ret = 1; return 1; } if (p.start != 0) { const struct grub_partition_map *partmap; --- grub_trunk/partmap/bsdlabel.c 2010-05-28 01:50:39.000000000 +0200 +++ grub_trunk_new/partmap/bsdlabel.c 2010-05-28 02:10:26.000000000 +0200 @@ -47,39 +47,46 @@ bsdlabel_partition_map_iterate (grub_dis /* Check if it is valid. */ if (label.magic != grub_cpu_to_le32 (GRUB_PC_PARTITION_BSD_LABEL_MAGIC)) return grub_error (GRUB_ERR_BAD_PART_TABLE, "no signature"); pos = sizeof (label) + GRUB_PC_PARTITION_BSD_LABEL_SECTOR * GRUB_DISK_SECTOR_SIZE; 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; p.offset = pos / GRUB_DISK_SECTOR_SIZE; p.index = pos % GRUB_DISK_SECTOR_SIZE; 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) + 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; } /* Partition map type. */ static struct grub_partition_map grub_bsdlabel_partition_map = { .name = "bsd",
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
