Syslinux - May 2010 - [PATCH] com32/lib/syslinux/load_linux.c: cmdline truncated

Hi,

in syslinux-386, I think I've found a problem in
   com32/lib/syslinux/load_linux.c
that leads to cmdline being truncated unnecessarily when e.g.
linux.c32 is used.

The patch below fixes the problem for me. But I don't know,
whether the patch is safe in all cases. I tried to reproduce
the logic found in runkernel.inc, but I might have missed
something. So, feel free to tell me what's wrong with it.

Please CC me, as I'm not on the list.

Thank you,
Bodo

------------------------------------------------------------

From: Bodo Stroesser <bstroesser at ts.fujitsu.com>

If a com32 modules uses load_linux, this handles long command
lines differently to core programs which include runkernel.inc
Thus linux.c32 loaded under pxelinux.0 truncates the cmdline
for linux 2.6.16 (SLES11) to max. 255 characters while loading
the same kernel from pxelinux.0 directly, cmdline will be truncated
at 2047 characters (+ closing 0).

This patch changes load_linux.c to use the same limits as
runkernel.inc does.

Signed-off-by: Bodo Stroesser <bstroesser at ts.fujitsu.com>
----

--- a/com32/lib/syslinux/load_linux.c	2010-05-18 23:00:11.000000000 +0200
+++ b/com32/lib/syslinux/load_linux.c	2010-05-18 23:10:15.000000000 +0200
@@ -289,8 +289,15 @@ int syslinux_boot_linux(void *kernel_buf
     if (hdr.version < 0x0205 || !(hdr.loadflags & LOAD_HIGH))
 	hdr.relocatable_kernel = 0;
 
-    if (hdr.version < 0x0206)
+    if (hdr.version < 0x0202)
 	hdr.cmdline_max_len = 256;
+    else if (hdr.version < 0x0206)
+	/* if (!(hdr.loadflags & 0x01))
+	 *      hdr.cmdline_max_len = 2048; // protocol limit
+	 * else
+	 *      hdr.cmdline_max_len = 2048; // current kernel buffer size
+	 */
+	hdr.cmdline_max_len = 2048;
 
     if (cmdline_size > hdr.cmdline_max_len) {
 	cmdline_size = hdr.cmdline_max_len;

On 05/19/2010 08:40 AM, Bodo Stroesser wrote:
> Hi,
> 
> in syslinux-386, I think I've found a problem in
>    com32/lib/syslinux/load_linux.c
> that leads to cmdline being truncated unnecessarily when e.g.
> linux.c32 is used.
> 
> The patch below fixes the problem for me. But I don't know,
> whether the patch is safe in all cases. I tried to reproduce
> the logic found in runkernel.inc, but I might have missed
> something. So, feel free to tell me what's wrong with it.
> 
> Please CC me, as I'm not on the list.
> 

No, this is actively wrong.

There is nothing with versions 2.03, .04 or .05 which makes it safe to
use 2048 bytes.  I have no idea what SLES11 did, but it sounds like a
local change.

	-hpa

On 05/19/2010 07:43 PM, H. Peter Anvin wrote:
>On 05/19/2010 08:40 AM, Bodo Stroesser wrote:
>
>> Hi,
>> 
>> in syslinux-386, I think I've found a problem in
>>    com32/lib/syslinux/load_linux.c
>> that leads to cmdline being truncated unnecessarily when e.g.
>> linux.c32 is used.
>> 
>> The patch below fixes the problem for me. But I don't know,
>> whether the patch is safe in all cases. I tried to reproduce
>> the logic found in runkernel.inc, but I might have missed
>> something. So, feel free to tell me what's wrong with it.
>> 
>> Please CC me, as I'm not on the list.
>> 
>
>
>No, this is actively wrong.
>
>There is nothing with versions 2.03, .04 or .05 which makes it safe to
>use 2048 bytes.  I have no idea what SLES11 did, but it sounds like a
>local change.
>
>	-hpa
So my patch is bad, sorry.

On the other hand, shouldn't core/runkernel.inc behave the same in
truncating command line? If I understand the code correctly, it doesn't.

When booting the same kernel (02.04) and a 400 chars command line twice,
once via pxelinux.0 directly, second time via pxelinux.0 and linux.c32,
the first time it doesn't truncate the command line while the second time
it does at 255 + 1 chars.

Bodo

On 05/21/2010 06:02 PM, H. Peter Anvin wrote:
>The kernel actually checks the command line length independently, and so
>feeding it a too-long line causes truncation.  However, people on this
>list complained that silent truncation was undesirable (of course, I'm
>not sure that linux.c32 actually prints anything in that case, either.)
No, it doesn't.
>
>Some distros extended the line size unilaterally, I understand -- there
>just isn't a way for the boot loader to know.
In kernel includes of the distro we are using I found
	#define COMMAND_LINE_SIZE 2048
So, for *us* my patch should be o.k., right?
>
>So there is basically the option to "shoot in the dark and hope for the
>best", or be conservative...
Oh, I see.
Just different tools from the same syslinux package using different
strategies here seems to be a bit confusing.
>
>	-hpa
Thank you
Bodo