Re: [Fwd: Re: Bug#495949: grub-common: grub-probe segfaults]

[Pavel Roskin <proski@gnu.org>]

On Fri, 2009-07-24 at 16:09 -0400, Pavel Roskin wrote:
> On Fri, 2009-07-24 at 20:46 +0200, Felix Zielcke wrote:
> > And another bug forward
> > Anyone has an idea why a dm-crypt/lvm leads to a segfault in the strcmp
> > here:
> >  grub_partition_iterate (dest_dev->disk, (strcmp (dest_partmap, "pc_partition_map") ?
> >                                            find_usable_region_gpt : find_usable_region_msdos));
> 
> dest_partmap is only assigned a value in identify_partmap.  If
> grub_partition_iterate() doesn't find any partitions, dest_partmap
> remains a random pointer.
> 
> The fix would be probably to initialize dest_partmap with NULL.  If it
> becomes "pc_partition_map", iterate with find_usable_region_msdos, if it
> becomes "gpt_partition_map", iterate with find_usable_region_gpt.  If
> it's NULL or another string, exit with a warning.

How about this?

Require positive identification of PC or GPT partition for embedding

ChangeLog:

	* util/i386/pc/grub-setup.c (setup): Initialize dest_partmap
	before iteration.  Don't allow embedding unless dest_partmap is
	"pc_partition_map" or "gpt_partition_map".
---
 util/i386/pc/grub-setup.c |   37 ++++++++++++++++++++++++++++++-------
 1 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c
index 5a51964..7ac5ace 100644
--- a/util/i386/pc/grub-setup.c
+++ b/util/i386/pc/grub-setup.c
@@ -329,16 +329,39 @@ setup (const char *dir,
       dest_partmap = p->partmap->name;
       return 1;
     }
+
+  dest_partmap = NULL;
   grub_partition_iterate (dest_dev->disk, identify_partmap);
 
-  grub_partition_iterate (dest_dev->disk, (strcmp (dest_partmap, "pc_partition_map") ?
-					   find_usable_region_gpt : find_usable_region_msdos));
-  if (embed_region.end == embed_region.start)
+  if (! dest_partmap)
     {
-      if (! strcmp (dest_partmap, "pc_partition_map"))
-	grub_util_warn ("This msdos-style partition label has no post-MBR gap; embedding won't be possible!");
-      else
-	grub_util_warn ("This GPT partition label has no BIOS Boot Partition; embedding won't be possible!");
+      grub_util_warn ("Cannot identify partition map.");
+      goto unable_to_embed;
+    }
+  else if (strcmp (dest_partmap, "pc_partition_map") == 0)
+    {
+      grub_partition_iterate (dest_dev->disk, find_usable_region_msdos);
+      if (embed_region.end == embed_region.start)
+	{
+	  grub_util_warn ("This msdos-style partition label has no post-MBR "
+			  "gap; embedding won't be possible!");
+	  goto unable_to_embed;
+	}
+    }
+  else if (strcmp (dest_partmap, "gpt_partition_map") == 0)
+    {
+      grub_partition_iterate (dest_dev->disk, find_usable_region_gpt);
+      if (embed_region.end == embed_region.start)
+	{
+	  grub_util_warn ("This GPT partition label has no BIOS Boot "
+			  "Partition; embedding won't be possible!");
+	  goto unable_to_embed;
+	}
+    }
+  else
+    {
+      grub_util_warn ("Embedding on partition type %s is unsupported",
+		      dest_partmap);
       goto unable_to_embed;
     }
 

-- 
Regards,
Pavel Roskin