Bug#550682: segfault in grub_device_iterate on 64bit platforms

[Dan Merillat]

Yo Grub2, I'm really happy for you and I'mma let you finish but...
allocating sizeof(p->next) instead of sizeof(*p) is the worst idiom of 
all time.  OF ALL TIME.

</Kayne West>

Diff against debian's 1.98-experimental-20100111, to which I submit the 
following patches:

diff --git a/kern/device.c b/kern/device.c
index d4de496..3727ddc 100644
--- a/kern/device.c
+++ b/kern/device.c
@@ -139,7 +139,7 @@ grub_device_iterate (int (*hook) (const char *name))
        if (! partition_name)
  	return 1;

-      p = grub_malloc (sizeof (p->next));
+      p = grub_malloc (sizeof (*p));
        if (!p)
  	{
  	  grub_free (partition_name);



Seriously though, where did someone see sizeof(p->next) and think it was 
a valid idiom?  It allocates a pointer, not a structure.  I've never 
seen it used before, and a quick rgrep shows it to be an anomaly in the 
grub source as well.  It happens to work on 32bit processors for tiny 
structures because the default alignment for malloc is 8 bytes - and 
struct part_ent is two pointers.  You end up overflowing into the 
padding and not trashing anything.

On 64bit, the structure is 16 bytes long, and trashes whatever the next 
allocation is, which leads to nasty problems and random crashes later.

Electric Fence.  Learn it, love it, live it.

And here's a freebie to avoid a null pointer deref that -lefence pointed 
out:

diff --git a/util/grub-probe.c b/util/grub-probe.c
index 4ba8a98..acb0887 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -94,6 +94,8 @@ probe_partmap (grub_disk_t disk)
  static int
  probe_raid_level (grub_disk_t disk)
  {
+  if (!disk)
+	  return -1;
    if (disk->dev->id != GRUB_DISK_DEVICE_RAID_ID)
      return -1;


Someone may want to look into why probe_raid_level can get passed an 
invalid disk pointer, but the combo of these two patches makes it work 
properly.  I don't know if it's possible to have a disk pointer without 
a dev pointer, but that's a question for people with some deeper knowledge.

This may fix some of the other bug reports - I see stacktraces crossing
grub_device_iterate which tells me that this is a potential corruption 
in all of them.