[BUGFIX] pc_partition_map and grub_setup

[BUGFIX] pc_partition_map and grub_setup

[Vladimir Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 439 bytes --]


I tried to install GRUB2 to a partition and found a bug in pc_partition_map:
in pc_partition_map_iterate pcdata was a local variable and its address was
written to p and p was returned.

2005-08-08 Vladimir Serbinenko <phcoder@gmail.com>
   
    * partmap/pc.c (pc_partition_map_iterate): pcdata is now dynamically
allocated
    * util/i386/pc/grub-setup.c (main): initialize prefix and dest_dev
to NULL
      prefix was freed twice
   


[-- Attachment #2: bugfix.patch --]
[-- Type: text/x-patch, Size: 3996 bytes --]

diff -urpN ./grub2_1/partmap/pc.c ./grub2_2/partmap/pc.c
--- ./grub2_1/partmap/pc.c	2005-08-08 11:34:22.000000000 +0200
+++ ./grub2_2/partmap/pc.c	2005-08-08 12:30:59.000000000 +0200
@@ -96,7 +96,7 @@ pc_partition_map_iterate (grub_disk_t di
 			  int (*hook) (const grub_partition_t partition))
 {
   struct grub_partition p;
-  struct grub_pc_partition pcdata;
+  struct grub_pc_partition *pcdata;
   struct grub_pc_partition_mbr mbr;
   struct grub_pc_partition_disk_label label;
   struct grub_disk raw;
@@ -105,10 +105,14 @@ pc_partition_map_iterate (grub_disk_t di
   raw = *disk;
   raw.partition = 0;
   
+  pcdata = (struct grub_pc_partition *) grub_malloc (sizeof (struct grub_pc_partition));
+  if (!pcdata)
+    goto finish;
+
   p.offset = 0;
-  pcdata.ext_offset = 0;
-  pcdata.dos_part = -1;
-  p.data = &pcdata;
+  pcdata->ext_offset = 0;
+  pcdata->dos_part = -1;
+  p.data = pcdata;
   p.partmap = &grub_pc_partition_map;
   
   while (1)
@@ -131,19 +135,19 @@ pc_partition_map_iterate (grub_disk_t di
 	  
 	  p.start = p.offset + grub_le_to_cpu32 (e->start);
 	  p.len = grub_le_to_cpu32 (e->length);
-	  pcdata.bsd_part = -1;
-	  pcdata.dos_type = e->type;
-	  pcdata.bsd_type = -1;
+	  pcdata->bsd_part = -1;
+	  pcdata->dos_type = e->type;
+	  pcdata->bsd_type = -1;
 
 	  grub_dprintf ("partition",
 			"partition %d: flag 0x%x, type 0x%x, start 0x%lx, len 0x%lx\n",
-			p.index, e->flag, pcdata.dos_type, p.start, p.len);
+			p.index, e->flag, pcdata->dos_type, p.start, p.len);
 
 	  /* If this partition is a normal one, call the hook.  */
 	  if (! grub_pc_partition_is_empty (e->type)
 	      && ! grub_pc_partition_is_extended (e->type))
 	    {
-	      pcdata.dos_part++;
+	      pcdata->dos_part++;
 	      
 	      if (hook (&p))
 		goto finish;
@@ -171,16 +175,16 @@ pc_partition_map_iterate (grub_disk_t di
 		    return grub_error (GRUB_ERR_BAD_PART_TABLE,
 				       "invalid disk label magic");
 
-		  for (pcdata.bsd_part = 0;
-		       pcdata.bsd_part < grub_cpu_to_le16 (label.num_partitions);
-		       pcdata.bsd_part++)
+		  for (pcdata->bsd_part = 0;
+		       pcdata->bsd_part < grub_cpu_to_le16 (label.num_partitions);
+		       pcdata->bsd_part++)
 		    {
 		      struct grub_pc_partition_bsd_entry *be
-			= label.entries + pcdata.bsd_part;
+			= label.entries + pcdata->bsd_part;
 
 		      p.start = grub_le_to_cpu32 (be->offset);
 		      p.len = grub_le_to_cpu32 (be->size);
-		      pcdata.bsd_type = be->fs_type;
+		      pcdata->bsd_type = be->fs_type;
 		      
 		      if (be->fs_type != GRUB_PC_PARTITION_BSD_TYPE_UNUSED)
 			if (hook (&p))
@@ -188,10 +192,10 @@ pc_partition_map_iterate (grub_disk_t di
 		    }
 		}
 	    }
-	  else if (pcdata.dos_part < 4)
+	  else if (pcdata->dos_part < 4)
 	    /* If this partition is a logical one, shouldn't increase the
 	       partition number.  */
-	    pcdata.dos_part++;
+	    pcdata->dos_part++;
 	}
 
       /* Find an extended partition.  */
@@ -201,9 +205,9 @@ pc_partition_map_iterate (grub_disk_t di
 	  
 	  if (grub_pc_partition_is_extended (e->type))
 	    {
-	      p.offset = pcdata.ext_offset + grub_le_to_cpu32 (e->start);
-	      if (! pcdata.ext_offset)
-		pcdata.ext_offset = p.offset;
+	      p.offset = pcdata->ext_offset + grub_le_to_cpu32 (e->start);
+	      if (! pcdata->ext_offset)
+		pcdata->ext_offset = p.offset;
 
 	      break;
 	    }
diff -urpN ./grub2_1/util/i386/pc/grub-setup.c ./grub2_2/util/i386/pc/grub-setup.c
--- ./grub2_1/util/i386/pc/grub-setup.c	2005-08-08 11:34:22.000000000 +0200
+++ ./grub2_2/util/i386/pc/grub-setup.c	2005-08-08 12:19:00.000000000 +0200
@@ -526,8 +526,8 @@ main (int argc, char *argv[])
   char *dir = 0;
   char *dev_map = 0;
   char *root_dev = 0;
-  char *prefix;
-  char *dest_dev;
+  char *prefix = 0;
+  char *dest_dev = 0;
   
   progname = "grub-setup";
 
@@ -682,7 +682,6 @@ main (int argc, char *argv[])
   free (dir);
   free (dev_map);
   free (root_dev);
-  free (prefix);
   free (dest_dev);
   
   return 0;

Re: [BUGFIX] pc_partition_map and grub_setup

[Yoshinori K. Okuji]

On Monday 08 August 2005 13:16, Vladimir Serbinenko wrote:
> I tried to install GRUB2 to a partition and found a bug in
> pc_partition_map: in pc_partition_map_iterate pcdata was a local variable
> and its address was written to p and p was returned.

I'm sorry. Actually I forgot to commit the fix in the pc partition map. I will 
apply your patch for grub-setup ASAP.

Thanks,
Okuji