From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1OWez6-0004ks-1U for mharc-grub-devel@gnu.org; Wed, 07 Jul 2010 20:29:00 -0400 Received: from [140.186.70.92] (port=44300 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OWez3-0004kM-HB for grub-devel@gnu.org; Wed, 07 Jul 2010 20:28:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OWez1-0004IH-Gp for grub-devel@gnu.org; Wed, 07 Jul 2010 20:28:57 -0400 Received: from mail-fx0-f41.google.com ([209.85.161.41]:35034) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OWez1-0004IB-9Q for grub-devel@gnu.org; Wed, 07 Jul 2010 20:28:55 -0400 Received: by fxm20 with SMTP id 20so120636fxm.0 for ; Wed, 07 Jul 2010 17:28:53 -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:references:in-reply-to :x-enigmail-version:content-type; bh=KWSmmIUN1PJoyUhhdfEDPe6U8lgVKaJ39VWvcLV4wG4=; b=INHOpjvCfd+8rO5O8lrFw9yKku8vAOnmjOjGr/Lq843Kdvkk88hHU+R3dUtTUIyc7z e0OGkwBP+qriimoEOKMavhSDQcPA/l/U2uvy+4cXSeegOzEZJ8TMXI3UUge5DaKLaZXn c3JvF8Jrt1D6XO2uTzR1gUuruEdTl/86TZ4uw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:x-enigmail-version:content-type; b=cBlvhLHOXFZihDhnpMxPiNkhlJCeBH2QsvEPtsDA7e0wdyN5hG3LjC6YTrvtziqeiz psme9b5Vzsqccegds4LdD/XAHabwvYV3Uss6ZMcEB7e0c2PqxTtBCocm7vUVXX8bLh5H nvQuUFwRyntQy8gcOYfM8NtnN4yvbs+NvNn+0= Received: by 10.223.123.79 with SMTP id o15mr6474546far.57.1278548933518; Wed, 07 Jul 2010 17:28:53 -0700 (PDT) Received: from debian.bg45.phnet (vpn-global-240-dhcp.ethz.ch [129.132.211.240]) by mx.google.com with ESMTPS id z16sm15639349fan.24.2010.07.07.17.28.51 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 07 Jul 2010 17:28:52 -0700 (PDT) Message-ID: <4C351BB9.9020802@gmail.com> Date: Thu, 08 Jul 2010 02:28:41 +0200 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.10) Gecko/20100620 Icedove/3.0.5 MIME-Version: 1.0 To: grub-devel@gnu.org References: <4BFF1337.3020100@gmail.com> <4C040166.3030502@gmail.com> <4C33A65A.7090005@gmail.com> In-Reply-To: <4C33A65A.7090005@gmail.com> X-Enigmail-Version: 1.0.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig1427B88D36D22AA67AC8F497" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: [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: Thu, 08 Jul 2010 00:28:59 -0000 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig1427B88D36D22AA67AC8F497 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/06/2010 11:55 PM, Gr=C3=A9goire Sutre wrote: > Hi, > > This is the reworked version of the patch. > >> There are few ramifications of this patch. First of all some partition= s >> which are just barely outside of the host partition will lead to >> something like "partition not found" errors in grub-probe. This messag= e >> 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", probab= ly >> such a warning in grub proper would be too annoying though). > > There is now a grub_dprintf where the partition is discarded. > As I already told you in real dprintf isn't seen by user. One need to grub_dprintf (....); #ifdef GRUB_UTIL grub_util_warn (...); #endif >> 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= =2E > You're right. This means we can't screw this test to save space. In this case it's better to do the complete check for early bug catch. >>> 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). > OK > =3D=3D=3D 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 @@ > =20 > grub_partition_map_t grub_partition_map_list; > =20 > +/* > + * 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 =3D grub_partition_get_len (disk->partition); > + else > + parent_len =3D disk->total_sectors; > =20 [grub_disk_get_len does exactly that] We shouldn't check for partitions being outside of disk since BIOS disk size limitations are common. Consider following situation: (hd0,msdos1,bsd1) /boot BIOS LIMIT (hd0,msdos1,bsd2) / This system is perfectly capable of booting but with your patch it won't.= We must always exercice best effort strategy. If something can bee booted, boot it. We should warn if a used final-nestedness partition is partialy outside the limit. Simple message usually scrolls way too fast and so usually ignored (if someone sees boot process at all). Perhaps we need a way to pass such warnings to kernel which then can take appropriate action (e.g. notify sysadmin) > + > + return (partition->start + partition->len <=3D 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 > =20 > auto int find_func (grub_disk_t d, const grub_partition_t partition)= ; > =20 > - int find_func (grub_disk_t d __attribute__ ((unused)), > + int find_func (grub_disk_t dsk, > const grub_partition_t partition) > { > - if (partnum =3D=3D partition->number) > - { > - p =3D (grub_partition_t) grub_malloc (sizeof (*p)); > - if (! p) > - return 1; > + if (partnum !=3D partition->number) > + return 0; > =20 > - 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, > +=09 Ditto > "out of range"); > + return 0; > } > =20 > - return 0; > + p =3D (grub_partition_t) grub_malloc (sizeof (*p)); > + if (! p) > + return 1; > + > + grub_memcpy (p, partition, sizeof (*p)); > + return 1; > } > =20 > partmap->iterate (disk, find_func); > @@ -138,6 +161,15 @@ grub_partition_iterate (struct grub_disk > const grub_partition_t partition) > { > struct grub_partition p =3D *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 =3D dsk->partition; > dsk->partition =3D 0; > if (hook (dsk, &p)) > > =3D=3D=3D 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 > =20 > for (p.number =3D 0; > p.number < grub_cpu_to_le16 (label.num_partitions); > - p.number++) > + p.number++, pos +=3D sizeof (struct grub_partition_bsd_entry)) > { > struct grub_partition_bsd_entry be; > =20 > @@ -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; > =20 > - p.start =3D grub_le_to_cpu32 (be.offset) - delta; > + p.start =3D grub_le_to_cpu32 (be.offset); > + if (p.start < delta) > + { > + grub_dprintf ("partition", > + "partition %d: invalid start (found 0x%llx, wanted >=3D 0x%llx)\n",= > + p.number, > + (unsigned long long) p.start, > + (unsigned long long) delta); > =20 grub_util_warn is needed here too. > + continue; > + } > + p.start -=3D delta; > p.len =3D grub_le_to_cpu32 (be.size); > p.partmap =3D &grub_bsdlabel_partition_map; > =20 > + 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 !=3D GRUB_PC_PARTITION_BSD_TYPE_UNUSED) > if (hook (disk, &p)) > return grub_errno; > - > - pos +=3D sizeof (struct grub_partition_bsd_entry); > } > =20 > return GRUB_ERR_NONE; > > =20 > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > =20 --=20 Regards Vladimir '=CF=86-coder/phcoder' Serbinenko --------------enig1427B88D36D22AA67AC8F497 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iF4EAREKAAYFAkw1G7kACgkQNak7dOguQgk7KQEAjyuVYvX3oMLXV6oLEDDSN2C0 fMIBSY0kJLId8BmIN10A/35zhUz0qE/PMOENdNwHgEncNxI+9VP3E/XC7vUjpYwz =2Q2h -----END PGP SIGNATURE----- --------------enig1427B88D36D22AA67AC8F497--