Hello Matt Kraai,
I noticed that you didn't move the "free(new_prefix)" line
to the end of the function. The net effect of not doing this
is that you might use the "new_prefix" pointer later even
though you "free()"d it before.
Attached is the patch which includes a fix for this problem.
It frees the new_prefix pointer at the end of the function
only if it is non-NULL.
I really appreciate the fact that you were able to find a
solution to the bug.
Regards,
M. Vefa Bicakci
--- grub2-1.98+20100702/util/i386/pc/grub-setup.c 2010-07-04 17:18:00.000000000 +0300
+++ grub2-1.98+20100702.fix/util/i386/pc/grub-setup.c 2010-07-04 17:21:26.000000000 +0300
@@ -101,6 +101,7 @@
grub_int32_t *install_dos_part, *install_bsd_part;
grub_int32_t dos_part, bsd_part;
char *prefix;
+ char *new_prefix = NULL;
char *tmp_img;
int i;
grub_disk_addr_t first_sector;
@@ -312,13 +313,11 @@
if (prefix[0] != '(')
{
- char *root_part_name, *new_prefix;
+ char *root_part_name;
root_part_name =
grub_partition_get_name (root_dev->disk->partition);
new_prefix = xasprintf ("(,%s)%s", root_part_name, prefix);
- strcpy (prefix, new_prefix);
- free (new_prefix);
free (root_part_name);
}
}
@@ -398,6 +397,9 @@
*install_dos_part = grub_cpu_to_le32 (dos_part);
*install_bsd_part = grub_cpu_to_le32 (bsd_part);
+ if (new_prefix != NULL)
+ strcpy(prefix, new_prefix);
+
/* The first blocklist contains the whole sectors. */
first_block->start = grub_cpu_to_le64 (embed_region.start + 1);
@@ -415,7 +417,7 @@
/* Write the core image onto the disk. */
if (grub_disk_write (dest_dev->disk, embed_region.start, 0, core_size, core_img))
grub_util_error ("%s", grub_errmsg);
-
+
/* FIXME: can this be skipped? */
*boot_drive = 0xFF;
@@ -572,6 +574,9 @@
*install_dos_part = grub_cpu_to_le32 (dos_part);
*install_bsd_part = grub_cpu_to_le32 (bsd_part);
+ if (new_prefix != NULL)
+ strcpy(prefix, new_prefix);
+
/* Write the first two sectors of the core image onto the disk. */
grub_util_info ("opening the core image `%s'", core_path);
fp = fopen (core_path, "r+b");
@@ -590,6 +595,9 @@
/* Sync is a Good Thing. */
sync ();
+ if (new_prefix != NULL)
+ free(new_prefix);
+
free (core_path);
free (core_img);
free (boot_img);
