From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Tb0WS-00033E-9a for mharc-grub-devel@gnu.org; Tue, 20 Nov 2012 21:58:44 -0500 Received: from eggs.gnu.org ([208.118.235.92]:42013) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tb0WP-00032k-II for grub-devel@gnu.org; Tue, 20 Nov 2012 21:58:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tb0WO-0004sB-AJ for grub-devel@gnu.org; Tue, 20 Nov 2012 21:58:41 -0500 Received: from mail-lb0-f169.google.com ([209.85.217.169]:52565) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tb0WO-0004rx-12 for grub-devel@gnu.org; Tue, 20 Nov 2012 21:58:40 -0500 Received: by mail-lb0-f169.google.com with SMTP id gk1so5717131lbb.0 for ; Tue, 20 Nov 2012 18:58:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:subject:message-id:x-mailer:mime-version:content-type :content-transfer-encoding; bh=fPoqclSQJg+DAjSA8c/OGVtl1RfYJtEAFKb7IJOXAfw=; b=l4wJnD4c2s3E4q+sUMF0SXWgynbW168AG9sZfnGqPa5C4NMmxN9KyJag3cYOVqRYAm lRdaYmkicyDCKLq/U3QKs16Kjq5CERixpdp9JMtlALTRHOuIEqRqJUCfGMhs3sSkU+el 5QWRA1TjTRqNso9uBtN9hPs89PzceylUmxNxmFhVy4XG19VJAS/9eLKlF2xHYgvshuuP kvtfSO57kpqHQ5W+kHOE7q4+PuH5TynbBTX9U8djhikmC+sLBQ/Wi/aBgs5u+2d8ZgoI OjNm+KV+Tp5YPvAyXXPDsc/HyDmiyTRJfn61g45QZx7oybliGivo6YzmQiSZrXmRHF7g O4xQ== Received: by 10.152.106.237 with SMTP id gx13mr16112886lab.46.1353466717706; Tue, 20 Nov 2012 18:58:37 -0800 (PST) Received: from opensuse.site (ppp83-237-28-2.pppoe.mtu-net.ru. [83.237.28.2]) by mx.google.com with ESMTPS id hu6sm5467980lab.13.2012.11.20.18.58.35 (version=SSLv3 cipher=OTHER); Tue, 20 Nov 2012 18:58:36 -0800 (PST) Date: Wed, 21 Nov 2012 06:58:33 +0400 From: Andrey Borzenkov To: grub-devel@gnu.org Subject: Should LDM check be less aggressive? Message-ID: <20121121065833.7497e7e8@opensuse.site> X-Mailer: Claws Mail 3.8.1 (GTK+ 2.24.10; x86_64-suse-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 209.85.217.169 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 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: Wed, 21 Nov 2012 02:58:42 -0000 There are multiple reports of GRUB2 refusing to install on disk which was used for LDM in the past. Example is: https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1061255 I tested behavior of Windows and Linux on a disk with valid LDM signature but manually modified partition table. Both ignore LDM labels if disk does not contain SFS partition (0x42) and treat is as pure MBR disk. This differs from GRUB2 behavior which always checks for LDM label whether SFS partition exists or not. Would the following be appropriate? It adds additional check for partition 0x42 before checking for LDM labels. -andrey Index: grub-2.00/grub-core/disk/ldm.c =================================================================== --- grub-2.00.orig/grub-core/disk/ldm.c +++ grub-2.00/grub-core/disk/ldm.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -103,6 +104,34 @@ read_int (grub_uint8_t *in, grub_size_t return ret; } +static int +msdos_has_ldm_partition (grub_disk_t dsk) +{ + grub_err_t err; + int has_sfs = 0; + auto int hook (grub_disk_t disk, const grub_partition_t p); + int hook (grub_disk_t disk __attribute__ ((unused)), const grub_partition_t p) + { + if (p->number >= 4) + return 1; + if (p->msdostype == GRUB_PC_PARTITION_TYPE_SFS) + { + has_sfs = 1; + return 1; + } + return 0; + } + + err = grub_partition_msdos_iterate (dsk, hook); + if (err) + { + grub_errno = GRUB_ERR_NONE; + return 0; + } + + return has_sfs; +} + static const grub_gpt_part_type_t ldm_type = GRUB_GPT_PARTITION_TYPE_LDM; static grub_disk_addr_t @@ -756,17 +785,20 @@ grub_ldm_detect (grub_disk_t disk, { int i; + int has_sfs = msdos_has_ldm_partition (disk); for (i = 0; i < 3; i++) { grub_disk_addr_t sector = LDM_LABEL_SECTOR; switch (i) { case 0: + if (!has_sfs) + continue; sector = LDM_LABEL_SECTOR; break; case 1: /* LDM is never inside a partition. */ - if (disk->partition) + if (!has_sfs || disk->partition) continue; sector = grub_disk_get_size (disk); if (sector == GRUB_DISK_SIZE_UNKNOWN) @@ -867,6 +899,7 @@ int grub_util_is_ldm (grub_disk_t disk) { int i; + int has_sfs = msdos_has_ldm_partition (disk); for (i = 0; i < 3; i++) { grub_disk_addr_t sector = LDM_LABEL_SECTOR; @@ -876,11 +909,13 @@ grub_util_is_ldm (grub_disk_t disk) switch (i) { case 0: + if (!has_sfs) + continue; sector = LDM_LABEL_SECTOR; break; case 1: /* LDM is never inside a partition. */ - if (disk->partition) + if (!has_sfs || disk->partition) continue; sector = grub_disk_get_size (disk); if (sector == GRUB_DISK_SIZE_UNKNOWN) Index: grub-2.00/include/grub/msdos_partition.h =================================================================== --- grub-2.00.orig/include/grub/msdos_partition.h +++ grub-2.00/include/grub/msdos_partition.h @@ -43,6 +43,7 @@ #define GRUB_PC_PARTITION_TYPE_FAT16_LBA 0xe #define GRUB_PC_PARTITION_TYPE_WIN95_EXTENDED 0xf #define GRUB_PC_PARTITION_TYPE_PLAN9 0x39 +#define GRUB_PC_PARTITION_TYPE_SFS 0x42 #define GRUB_PC_PARTITION_TYPE_EZD 0x55 #define GRUB_PC_PARTITION_TYPE_MINIX 0x80 #define GRUB_PC_PARTITION_TYPE_LINUX_MINIX 0x81 Index: grub-2.00/Makefile.util.def =================================================================== --- grub-2.00.orig/Makefile.util.def +++ grub-2.00/Makefile.util.def @@ -32,6 +32,7 @@ library = { common = grub-core/disk/ldm.c; common = grub-core/disk/diskfilter.c; common = grub-core/partmap/gpt.c; + common = grub-core/partmap/msdos.c; }; library = { @@ -109,7 +110,6 @@ library = { common = grub-core/partmap/acorn.c; common = grub-core/partmap/amiga.c; common = grub-core/partmap/apple.c; - common = grub-core/partmap/msdos.c; common = grub-core/partmap/sun.c; common = grub-core/partmap/plan.c; common = grub-core/partmap/dvh.c;