THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit

Does anyone know the actual details of the LILO breakage?
If the problem is that LILO doesn't null-terminate the string when it's too long, then we can deal with this automatically, without introducing compile-time options (which were already once rejected.)

If the problem is with LILO booting *old* kernels, then that's going to have to require some LILO changes, and probably a boot revision bump.

Revert "x86_64/i386: increase command line size" patch It's a bootup dependancy, you can't just increase it randomly, and it breaks booting with LILO. Pointed out by Janos Farkas and Adrian Bunk.

Quoted 5 lines

Revert "x86_64/i386: increase command line size" patch It's a bootup dependancy, you can't just increase it randomly, and it breaks booting with LILO. Pointed out by Janos Farkas and Adrian Bunk.

Quoted 6 lines · Alon Bar-Lev

Quoted 5 lines

Revert "x86_64/i386: increase command line size" patch It's a bootup dependancy, you can't just increase it randomly, and it breaks booting with LILO. Pointed out by Janos Farkas and Adrian Bunk.

Can we get the details, please, instead of a repeat of the same patch, so we can figure out a proper solution?

-hpa

This should be a simple modification and I don't see why we should fight on the LILO problem (if exists) when we have the compile time config options alternative.

People who uses LILO may leave the default 256 value. Other may migrate to a higher one.

Quoted 5 lines · Alon Bar-Lev

This should be a simple modification and I don't see why we should fight on the LILO problem (if exists) when we have the compile time config options alternative. People who uses LILO may leave the default 256 value. Other may migrate to a higher one.

This is cargo-cult programming, sorry. Let's find out what the problem is and fix it right. If that involves fixing LILO and/or dealing with a LILO bug, we have ways we can do that.

-hpa

It is probably fairly easy to increase the LILO command line to 512 bytes (including terminator). Beyond 512 there are complicating factors:

1. "lilo -R ..." -- the space reserved for the stored command line is 1 sector.
2. configuration option "fallback" -- again 1 sector is the amount reserved.

There are 2 buffers used for the command line. Since these are allocated on sector boundaries, 512 should present no serious problems.

John Coffman wrote:
The problem isn't that LILO can't handle more than some number of characters; that's a LILO issue and doesn't affect the kernel.

The problem is that some people have reported that the kernel crashes if booted with LILO and the size limit is more than 255. They haven't so far commented on how they observed that, and that's a major problem.

If the issue is that LILO doesn't null-terminate overlong command lines, then that's pretty easy to deal with:

- If the kernel sees protocol version <= 2.01, limit is 255+null.
- If the kernel sees protocol version >= 2.02, but ID is 0x1X, limit is 255+null.
- Otherwise limit is higher.

When LILO is fixed, it has to bump the ID byte version number.

What ID byte values has LILO used?

Quoted 2 lines

The problem is that some people have reported that the kernel crashes if booted with LILO and the size limit is more than 255. They haven't so far commented on how they observed that, and that's a major problem.

Just re-compiling LILO with the COMMAND_LINE_SIZE parameter changed from 256 to 512 will not work. A .bss area must be moved to avoid clobbering the kernel header.

Just re-compiling LILO with the COMMAND_LINE_SIZE parameter changed from 256 to 512 will not work. A .bss area must be moved to avoid clobbering the kernel header.

From LILO code I can see that COMMAND_LINE_SIZE is defined

If the *kernel* limit is modified, but the LILO limit is not, what will happen? This is the real crux of the matter.

At 02:57 PM Friday 5/5/2006, H. Peter Anvin wrote:
Okay, let me ask this:

Quoted 1 line

If the *kernel* limit is modified, but the LILO limit is not, what will happen? This is the real crux of the matter.

The length of the kernel command line will be limited by the size of the boot loader buffer. LILO always inserts a NUL terminator.

--John

P.S. The LILO command line buffer has always been 1 sector (512 bytes); however, only the first half is actually used for the command line. No kernel can do any harm by setting "boot_cmdline[511] = 0;" for any version of LILO back to version 20 (and probably before).

Quoted 9 lines · John Coffman

At 02:57 PM Friday 5/5/2006, H. Peter Anvin wrote:
Okay, let me ask this:

Quoted 2 lines

If the *kernel* limit is modified, but the LILO limit is not, what will happen? This is the real crux of the matter.

The length of the kernel command line will be limited by the size of the boot loader buffer. LILO always inserts a NUL terminator.

Quoted 7 lines

--John

P.S. The LILO command line buffer has always been 1 sector (512 bytes); however, only the first half is actually used for the command line. No kernel can do any harm by setting "boot_cmdline[511] = 0;" for any version of LILO back to version 20 (and probably before).

Okay... **DOES ANYONE HAVE ANY ACTUAL EVIDENCE TO THE CONTRARY???**, and if so, **WHAT ARE THE DETAILS**?

All I've heard so far is hearsay. "X said that Y had said..."

-hpa

Extending the kernel parameters to a 2048 bytes for boot protocol >=2.02 of i386, ia64 and x86_64 architectures for linux-2.6.18-rc4-mm2.

Current implementation allows the kernel to receive up to 255 characters from the bootloader. In current environment, the command-line is used in order to specify many values, including suspend/resume, module arguments, splash, initramfs and more. 255 characters are not enough anymore.

The last time I tried this on x86-64 lilo on systems that used EDD broke. EDD uses part of the bootup page too. So most likely it's not that simple.

And please don't shout your subjects.

Quoted 6 lines · Andi Kleen

The last time I tried this on x86-64 lilo on systems that used EDD broke. EDD uses part of the bootup page too. So most likely it's not that simple.

And please don't shout your subjects.

On i386, the command line is never stored in the bootup page; only a pointer to it is. The copying is done straight into the saved_command_line buffer in the kernel BSS (head.S lines 79-104).

x86-64 does the same thing, but in C code (head64.c lines 45-56.) Thus, if you had a problem with LILO, I suspect the problem was inside LILO itself, and not a kernel issue.

Just increasing that constant caused various lilo setups to not boot anymore. I don't know who is actually to blame, just wanting to point out that this "obvious" patch isn't actually that obvious.

Just increasing that constant caused various lilo setups to not boot anymore. I don't know who is actually to blame, just wanting to point out that this "obvious" patch isn't actually that obvious.

-Andi

Quoted 5 lines · Andi Kleen

Just increasing that constant caused various lilo setups to not boot anymore. I don't know who is actually to blame, just wanting to point out that this "obvious" patch isn't actually that obvious.

How would that even be possible (unless you recompiled LILO with the new headers)? There would be no difference in the memory image at the point LILO hands off to the kernel.

In order to reproduce this we need some details about your "various LILO setups", or this will remain as a source of cargo cult programming.

Quoted 5 lines · H. Peter Anvin

Quoted 4 lines · Andi Kleen

Just increasing that constant caused various lilo setups to not boot anymore. I don't know who is actually to blame, just wanting to point out that this "obvious" patch isn't actually that obvious.

Quoted 1 line

How would that even be possible (unless you recompiled LILO with the new headers)? There would be no difference in the memory image at the point LILO hands off to the kernel.

AFAIK the problem was that some EDD data got overwritten.

Quoted 1 line

In order to reproduce this we need some details about your "various LILO setups", or this will remain as a source of cargo cult programming.

You can search the mailing list archives, it's all in there if you don't belive me.

lilo -T ebda

Quoted 11 lines · Andi Kleen

Quoted 5 lines · H. Peter Anvin

Quoted 3 lines · Andi Kleen

Just increasing that constant caused various lilo setups to not boot anymore. I don't know who is actually to blame, just wanting to point out that this "obvious" patch isn't actually that obvious.

How would that even be possible (unless you recompiled LILO with the new headers)? There would be no difference in the memory image at the point LILO hands off to the kernel.

AFAIK the problem was that some EDD data got overwritten.

Quoted 1 line

In order to reproduce this we need some details about your "various LILO setups", or this will remain as a source of cargo cult programming.

You can search the mailing list archives, it's all in there if you don't belive me.

Found the references. This seems to imply that EDD overwrites the area used by LILO 22.6.1. LILO 22.6.1 uses the new boot protocol, with the full pointer, and seems to obey the spec as far as I can read the code. I'm going to try to run it in simulation and observe the failure that way.

However, something is still seriously out of joint. The EDD data actually overlays the setup code, not the bootsect code, and thus there "shouldn't" be any way that this could interfere. My best guess at this time is that either the EDD code or LILO uses memory it's not supposed to use, and the simulation should hopefully reveal that.

Sorry if I seem snarky on this, but if we can't get to the bottom of this we can't ever fix it.

-hpa

Found the references. This seems to imply that EDD overwrites the area used by LILO 22.6.1. LILO 22.6.1 uses the new boot protocol, with the full pointer, and seems to obey the spec as far as I can read the code.
I'm going to try to run it in simulation and observe the failure that way.

However, something is still seriously out of joint. The EDD data actually overlays the setup code, not the bootsect code, and thus there "shouldn't" be any way that this could interfere. My best guess at this time is that either the EDD code or LILO uses memory it's not supposed to use, and the simulation should hopefully reveal that.

Sorry if I seem snarky on this, but if we can't get to the bottom of this we can't ever fix it.

-hpa

Quoted 8 lines · H. Peter Anvin

Found the references. This seems to imply that EDD overwrites the area used by LILO 22.6.1. LILO 22.6.1 uses the new boot protocol, with the full pointer, and seems to obey the spec as far as I can read the code. I'm going to try to run it in simulation and observe the failure that way.

However, something is still seriously out of joint. The EDD data actually overlays the setup code, not the bootsect code, and thus there "shouldn't" be any way that this could interfere. My best guess at this time is that either the EDD code or LILO uses memory it's not supposed to use, and the simulation should hopefully reveal that.

Sorry if I seem snarky on this, but if we can't get to the bottom of this we can't ever fix it.

-hpa

I think I've found one problem... But I it should not be the major one. The EDD code scans the command-line as fixed string.
What about something like the following?

Best Regards,
Alon Bar-Lev.

diff -urNp linux-2.6.18-rc4-mm2/arch/i386/boot/edd.S linux-2.6.18-rc4-mm2.new/arch/i386/boot/edd.S
--- linux-2.6.18-rc4-mm2/arch/i386/boot/edd.S 2006-06-18
04:49:35.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/arch/i386/boot/edd.S 2006-08-28
08:55:01.000000000 +0300

Better patch.
I've noticed that this code sets esi but then reference using si... So fixed to use esi (It worked so far since we are in low area... But I think using the same register type is cleaner...)

Totally pointless since we're in 16-bit mode (as is the "incl %esi")... I guess it's "better" in the sense that if we run out of that we'll crash due to a segment overrun... maybe (some BIOSes leave us unknowningly in big real mode...)

Quoted 4 lines · H. Peter Anvin

Totally pointless since we're in 16-bit mode (as is the "incl %esi")... I guess it's "better" in the sense that if we run out of that we'll crash due to a segment overrun... maybe (some BIOSes leave us unknowningly in big real mode...)

So leave as is? Loading address into esi and reference as si? Or modify the whole code to use 16 bits?

Quoted 6 lines · Alon Bar-Lev

Quoted 4 lines · H. Peter Anvin

Totally pointless since we're in 16-bit mode (as is the "incl %esi")... I guess it's "better" in the sense that if we run out of that we'll crash due to a segment overrun... maybe (some BIOSes leave us unknowningly in big real mode...)

Quoted 3 lines

So leave as is? Loading address into esi and reference as si? Or modify the whole code to use 16 bits?

Probably modifying the whole code to use 16 bits, unless there is a specific reason not to (Matt?)

No reason. I was just trying to be careful, not leaving data in the upper bits of those registers going uninitialized. If we know they're not being used ever, then it's not a problem. But I don't think that's the source of the command line size concern, is it?

No reason. I was just trying to be careful, not leaving data in the upper bits of those registers going uninitialized. If we know they're not being used ever, then it's not a problem. But I don't think that's the source of the command line size concern, is it?

Quoted 6 lines · Matt Domsch

No reason. I was just trying to be careful, not leaving data in the upper bits of those registers going uninitialized. If we know they're not being used ever, then it's not a problem. But I don't think that's the source of the command line size concern, is it?

No, it's treating the command line as a fixed buffer, as opposed to a null-terminated string. This was always a bug, by the way.

OK, I'll look at fixing that, and using %esi throughout.

Thanks,
Matt

Quoted 4 lines · Matt Domsch

No reason. I was just trying to be careful, not leaving data in the upper bits of those registers going uninitialized. If we know they're not being used ever, then it's not a problem. But I don't think that's the source of the command line size concern, is it?

Since the cmd_line_ptr is 32bit, we can load the lower 16bits into si, ignoring the upper 16bits, or we can use esi for all references. I think using esi for all references is cleaner...

Quoted 7 lines · H. Peter Anvin

Quoted 5 lines · Matt Domsch

No reason. I was just trying to be careful, not leaving data in the upper bits of those registers going uninitialized. If we know they're not being used ever, then it's not a problem. But I don't think that's the source of the command line size concern, is it?

No, it's treating the command line as a fixed buffer, as opposed to a null-terminated string. This was always a bug, by the way.

OK, I'll look at fixing that, and using %esi throughout.

Quoted 7 lines · H. Peter Anvin

Quoted 5 lines · Matt Domsch

No reason. I was just trying to be careful, not leaving data in the upper bits of those registers going uninitialized. If we know they're not being used ever, then it's not a problem. But I don't think that's the source of the command line size concern, is it?

No, it's treating the command line as a fixed buffer, as opposed to a null-terminated string. This was always a bug, by the way.

OK, I'll look at fixing that, and using %esi throughout.

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

The EDD code would scan the command line as a fixed array, without taking account of either whitespace, null-termination, the old command-line protocol, late overrides early, or the fact that the command line may not be reachable from INITSEG.

This should fix those problems, and enable us to use a longer command line.

Signed-off-by H. Peter Anvin <[email protected]>

arch/i386/boot/edd.S +8 -0

index 4b84ea2..03712a0 100644
 
+	andl	%esi, %esi
+	jz	old_cl			# Old boot protocol?
+
+# Convert to a real-mode pointer in fs:si
+	movl	%esi, %eax
+	shrl	$4, %eax
+	movw	%ax, %fs
+	andw	$0xf, %si

arch/i386/boot/edd.S +12 -0

+	jmp	done_cl			# Wraparound...
+
+cl_skipword:
+	movb	%fs:(%si), %al		# End of string?
+	andb	%al, %al
+	jz	done_cl
+	cmpb	$0x20, %al
+	jbe	cl_atspace
+	incw	%si
+	jnz	cl_skipword
+	jmp	done_cl			# Wraparound...
+	

arch/i386/boot/edd.S #include <linux/edd.h> · line 50

Quoted 9 lines

arch/i386/boot/edd.S +9 -0

+
+# Old-style boot protocol?
+old_cl:
+    push    %ds            # aka INITSEG
+    pop    %fs
+
+    cmpw    $0xa33f, (0x20)
+    jne    done_cl            # No command line at all?
+    movw    (0x22), %si        # Pointer relative to INITSEG

Perhaps you should convert ds:si to flat pointer and then this flat pointer to fs:si using method above, to avoid problems with dword access with si > 0xfffc or word access with si > 0xfffe ?

arch/i386/boot/edd.S #include <linux/edd.h> · line 57

Quoted 9 lines

arch/i386/boot/edd.S +6 -2

+
+# fs:si has the pointer to the command line now
+have_cl_pointer:
+   
 # loop through kernel command line one byte at a time
-cl_loop:
-    cmpl    $EDD_CL_EQUALS, (%si)
+cl_atspace:
+    movl    %fs:(%si), %eax

This looks fine for new boot protocol, but what if old boot protocol puts command line so that its last byte is at INITSEG:0xffff ? You get #GP here, then, although command line is correctly zero terminated and does not overflow segment.

If si is 0xfffb here, bad things happen. I know, things I've pointed out should not be problem in real world, and new code is definitely better than old one, but if you already have code to avoid endless loop if command line points to 64KB array of 0xFF let's do that right, no?

Extending the kernel parameters to a 2048 bytes for boot protocol >=2.02 of i386, ia64 and x86_64 architectures for linux-2.6.18-rc4-mm2.

Current implementation allows the kernel to receive up to 255 characters from the bootloader. In current environment, the command-line is used in order to specify many values, including suspend/resume, module arguments, splash, initramfs and more. 255 characters are not enough anymore.

EDD issue was fixed recently by H. Peter Anvin, please add this to mm so more problems may be found.

IA64 booting is completely different. I don't think it should be in this patch. At least you would need to check with the IA64 maintainer first.

And the other thing is that this will cost memory. Either make it dependend on !CONFIG_SMALL or fix the boot code to save the command line into a kmalloc'ed buffer of the right size and __init the original one

Quoted 5 lines

And the other thing is that this will cost memory. Either make it dependend on !CONFIG_SMALL or fix the boot code to save the command line into a kmalloc'ed buffer of the right size and __init the original one

I don't mind doing either... Any preference for one of them? The kmalloc approach seems nicer..

Quoted 8 lines · Alon Bar-Lev

Quoted 5 lines

And the other thing is that this will cost memory. Either make it dependend on !CONFIG_SMALL or fix the boot code to save the command line into a kmalloc'ed buffer of the right size and __init the original one

I don't mind doing either... Any preference for one of them? The kmalloc approach seems nicer..

kmalloc is better yes. You just have to do it after kmalloc is up and running and make sure the users before reference the __init'ed version. I suspect only /proc/cmdline will need the kmalloc version after booting, nobody else should look at the command line.

-Andi

Quoted 2 lines · On Wed, 30 Aug 2006 18:56:11 +0200 Andi Kleen <[email protected]> wrote:

IA64 booting is completely different. I don't think it should be in this patch. At least you would need to check with the IA64 maintainer first.

OK... no problem.

Quoted 2 lines

And the other thing is that this will cost memory. Either make
it dependend on !CONFIG_SMALL or fix the boot code to save the command line into a kmalloc'ed buffer of the right size and __init the original one

I don't mind doing either... Any preference for one of them? The kmalloc approach seems nicer..

This is not entirely true...
All architectures sets saved_command_line variable... So I can add __init to the saved_command_line and copy its contents into kmalloced persistence_command_line at main.c.

Quoted 7 lines · Alon Bar-Lev

This is not entirely true...
All architectures sets saved_command_line variable... So I can add __init to the saved_command_line and copy its contents into kmalloced persistence_command_line at main.c.

My opinion is that you should change saved_command_line (which already implies a copy) to be the kmalloc'd version and call the fixed-sized buffer something else.

Quoted 7 lines · H. Peter Anvin

Quoted 6 lines · Alon Bar-Lev

This is not entirely true...
All architectures sets saved_command_line variable... So I can add __init to the saved_command_line and copy its contents into kmalloced persistence_command_line at main.c.

Quoted 1 line

My opinion is that you should change saved_command_line (which already implies a copy) to be the kmalloc'd version and call the fixed-sized buffer something else.

It might be safer to rename everything. Then all users could be caught and audited. This would ensure saved_command_line is not accessed before the kmalloc'ed copy exists.

Disadvantage: more architectures to change.

Quoted 7 lines · Alon Bar-Lev

This is not entirely true...
All architectures sets saved_command_line variable... So I can add __init to the saved_command_line and copy its contents into kmalloced persistence_command_line at main.c.

My opinion is that you should change saved_command_line (which already implies a copy) to be the kmalloc'd version and call the fixed-sized buffer something else.

-hpa

Changing saved_command_line is a modification to all architectures... They all modify this variable...
So, should I submit a patch to all architectures? How can I test this?

Also for i386 the code is assembly... So I can modify the code to write into a __init buffer and then kmalloc in setup.c.