All of lore.kernel.org
 help / color / mirror / Atom feed
* Should LDM check be less aggressive?
@ 2012-11-21  2:58 Andrey Borzenkov
  2012-11-21 21:23 ` Phillip Susi
  2013-01-20 21:58 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 2 replies; 6+ messages in thread
From: Andrey Borzenkov @ 2012-11-21  2:58 UTC (permalink / raw)
  To: grub-devel

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 <grub/err.h>
 #include <grub/misc.h>
 #include <grub/diskfilter.h>
+#include <grub/msdos_partition.h>
 #include <grub/gpt_partition.h>
 #include <grub/i18n.h>
 
@@ -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;


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-05-28 15:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-21  2:58 Should LDM check be less aggressive? Andrey Borzenkov
2012-11-21 21:23 ` Phillip Susi
2013-01-20 21:58 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-01-21 18:50   ` Andrey Borzenkov
2013-05-28 14:34     ` Phillip Susi
2013-05-28 15:05       ` Andrey Borzenkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.