From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1OHnm3-0005YX-TB for mharc-grub-devel@gnu.org; Thu, 27 May 2010 20:50:07 -0400 Received: from [140.186.70.92] (port=55104 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OHnm0-0005Wh-7r for grub-devel@gnu.org; Thu, 27 May 2010 20:50:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OHnly-0002ag-Rz for grub-devel@gnu.org; Thu, 27 May 2010 20:50:04 -0400 Received: from mail-wy0-f169.google.com ([74.125.82.169]:52965) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OHnly-0002ZA-HJ for grub-devel@gnu.org; Thu, 27 May 2010 20:50:02 -0400 Received: by wyb40 with SMTP id 40so103308wyb.0 for ; Thu, 27 May 2010 17:50:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:subject:content-type; bh=9v0rDBntX5RwbsaMEbrdtkX7EKyRk13x+ZEzM6DVcDI=; b=jdyRHMqCiRw5TUYfVFCSeZX+sSJKy982p/CyHH7KcIRzxZzPJBFJnvyLY7hKcENkRe e/LuHkWVhRuOtE+Vrhp3O3S+1GlqBH7fPIRkXEVkS2kyRBnnf605LqecEaAmQzOipUmW qQSezxRGrBlzuiMtfG/88ivtS++HVEhfxDxb0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject :content-type; b=ZpaGi12kqsUoB9zgmL2/5r2PEPFd0/L+fPlMVwxFHXdtoPuXXBdTnihXmGxj1Hph0h ens3e12YBvAy8rWPpmq+CmAYPoUMKadP0Y9RhzjY4FU06WOwjKZ2MLY0VF98IsHz8bHs DP1CkfyYey07Y1cbb5ZUo0CLMZsDfulGXFPzM= Received: by 10.227.154.4 with SMTP id m4mr874606wbw.153.1275007800707; Thu, 27 May 2010 17:50:00 -0700 (PDT) Received: from [192.168.1.108] (c2433-1-88-160-112-182.fbx.proxad.net [88.160.112.182]) by mx.google.com with ESMTPS id u32sm10657946wbc.23.2010.05.27.17.49.59 (version=TLSv1/SSLv3 cipher=RC4-MD5); Thu, 27 May 2010 17:50:00 -0700 (PDT) Message-ID: <4BFF1337.3020100@gmail.com> Date: Fri, 28 May 2010 02:49:59 +0200 From: =?ISO-8859-1?Q?Gr=E9goire_Sutre?= User-Agent: Thunderbird 2.0.0.23 (X11/20091027) MIME-Version: 1.0 To: The development of GRUB 2 Content-Type: multipart/mixed; boundary="------------000100070403080506010803" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: [Patch] Discard incorrect nested partitions (fixes #29956) X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 May 2010 00:50:05 -0000 This is a multi-part message in MIME format. --------------000100070403080506010803 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit 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 --------------000100070403080506010803 Content-Type: text/x-patch; name="nesting.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="nesting.diff" --- 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 . */ #include #include #include #include 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", --------------000100070403080506010803--