Re: [bug #44239] Behavior of syslinux_parse differs from SYSLINUX for initrd= option

[Lunar <lunar@torproject.org>]

[-- Attachment #1.1: Type: text/plain, Size: 456 bytes --]

Andrei Borzenkov:
> Follow-up Comment #1, bug #44239 (project grub):
> 
> Can't comment about behavior itself. Re. patch
> 
> +	cmdline = grub_strdup(curentry->append);
> Error check missing.
> 
> +	grub_free(cmdline);
> You need to delete free(cmdline) later.

Both fixed.

> Again, can you post updated version to grub-devel for easier review?

Here it is.

-- 
Lunar                                             <lunar@torproject.org>

[-- Attachment #1.2: 0002-lib-syslinux_parse-keep-initrd-in-kernel-the-command.patch --]
[-- Type: text/x-diff, Size: 2644 bytes --]

From d1a88855eea4b2b3099d8ad8ff689305f74736cd Mon Sep 17 00:00:00 2001
From: Lunar <lunar@torproject.org>
Date: Sat, 14 Feb 2015 16:50:14 +0100
Subject: [PATCH 2/2] lib/syslinux_parse: keep initrd= in kernel the command
 line

SYSLINUX parses but does not remove "initrd=..." option from the kernel
command line. As some distributions (e.g. Tails) rely on its
presence, let's keep the same behavior.

Fixes Savannah bug #44239.
---
 grub-core/lib/syslinux_parse.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/grub-core/lib/syslinux_parse.c b/grub-core/lib/syslinux_parse.c
index b0d5f9e..1318c3a 100644
--- a/grub-core/lib/syslinux_parse.c
+++ b/grub-core/lib/syslinux_parse.c
@@ -939,6 +939,9 @@ write_entry (struct output_buffer *outbuf,
 	char *ptr;
 	char *cmdline;
 	char *initrd = NULL;
+	cmdline = grub_strdup(curentry->append ? curentry->append : "");
+	if (!cmdline)
+	  return grub_errno;
 	for (ptr = curentry->append; ptr && *ptr; ptr++)
 	  if ((ptr == curentry->append || grub_isspace (ptr[-1]))
 	      && grub_strncasecmp (ptr, "initrd=", sizeof ("initrd=") - 1)
@@ -946,32 +949,16 @@ write_entry (struct output_buffer *outbuf,
 	    break;
 	if (ptr && *ptr)
 	  {
-	    char *ptr2;
-	    grub_size_t totlen = grub_strlen (curentry->append);
 	    initrd = ptr + sizeof ("initrd=") - 1;
-	    for (ptr2 = ptr; *ptr2 && !grub_isspace (*ptr2); ptr2++);
-	    if (*ptr2)
-	      {
-		*ptr2 = 0;
-		ptr2++;
-	      }
-	    cmdline = grub_malloc (totlen + 1 - (ptr2 - ptr));
-	    if (!cmdline)
-	      return grub_errno;
-	    grub_memcpy (cmdline, curentry->append, ptr - curentry->append);
-	    grub_memcpy (cmdline + (ptr - curentry->append),
-			 ptr2, totlen - (ptr2 - curentry->append));
-	    *(cmdline + totlen - (ptr2 - ptr)) = 0;
+	    for (; *ptr && !grub_isspace (*ptr); ptr++);
+	    *ptr = '\0';
 	  }
-	else
-	  cmdline = curentry->append;
 	print_string (" if test x$grub_platform = xpc; then "
 		      "linux_suffix=16; else linux_suffix= ; fi\n");
 	print_string ("  linux$linux_suffix ");
 	print_file (outbuf, menu, curentry->kernel_file, NULL);
 	print_string (" ");
-	if (cmdline)
-	  print (outbuf, cmdline, grub_strlen (cmdline));
+	print (outbuf, cmdline, grub_strlen (cmdline));
 	print_string ("\n");
 	if (initrd || curentry->initrds)
 	  {
@@ -990,8 +977,7 @@ write_entry (struct output_buffer *outbuf,
 
 	    print_string ("\n");
 	  }
-	if (ptr && *ptr)
-	  grub_free (cmdline);
+	grub_free (cmdline);
       }
       break;
     case KERNEL_CHAINLOADER:
-- 
1.9.1


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]