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.
Signed-off-by: Alon Bar-Lev <[email protected]>
---
diff -urNp linux-2.6.18-rc4-mm2/include/asm-i386/param.h linux-2.6.18-rc4-mm2.new/include/asm-i386/param.h
--- linux-2.6.18-rc4-mm2/include/asm-i386/param.h 2006-08-25 16:10:56.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/include/asm-i386/param.h 2006-08-26 02:30:52.000000000 +0300
@@ -18,6 +18,5 @@
#endif
#define MAXHOSTNAMELEN 64 /* max length of hostname */
-#define COMMAND_LINE_SIZE 256
#endif
diff -urNp linux-2.6.18-rc4-mm2/include/asm-i386/setup.h linux-2.6.18-rc4-mm2.new/include/asm-i386/setup.h
--- linux-2.6.18-rc4-mm2/include/asm-i386/setup.h 2006-08-25 16:10:56.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/include/asm-i386/setup.h 2006-08-26 02:30:52.000000000 +0300
@@ -15,7 +15,7 @@
#define MAX_NONPAE_PFN (1 << 20)
#define PARAM_SIZE 4096
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048
#define OLD_CL_MAGIC_ADDR 0x90020
#define OLD_CL_MAGIC 0xA33F
diff -urNp linux-2.6.18-rc4-mm2/include/asm-ia64/setup.h linux-2.6.18-rc4-mm2.new/include/asm-ia64/setup.h
--- linux-2.6.18-rc4-mm2/include/asm-ia64/setup.h 2006-06-18 04:49:35.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/include/asm-ia64/setup.h 2006-08-26 02:30:52.000000000 +0300
@@ -1,6 +1,6 @@
#ifndef __IA64_SETUP_H
#define __IA64_SETUP_H
-#define COMMAND_LINE_SIZE 512
+#define COMMAND_LINE_SIZE 2048
#endif
diff -urNp linux-2.6.18-rc4-mm2/include/asm-x86_64/setup.h linux-2.6.18-rc4-mm2.new/include/asm-x86_64/setup.h
--- linux-2.6.18-rc4-mm2/include/asm-x86_64/setup.h 2006-06-18 04:49:35.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/include/asm-x86_64/setup.h 2006-08-26 02:32:44.000000000 +0300
@@ -1,6 +1,6 @@
#ifndef _x8664_SETUP_H
#define _x8664_SETUP_H
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048
#endif
Alon Bar-Lev <[email protected]> writes:
> 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.
-Andi
Andi Kleen wrote:
>
> 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.
-hpa
On Sunday 27 August 2006 20:50, H. Peter Anvin wrote:
> Andi Kleen wrote:
> >
> > 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.
-Andi
Andi Kleen wrote:
>
> 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.
-hpa
On 8/27/06, Andi Kleen <[email protected]> wrote:
> 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
>
Hello,
lilo has its own COMMAND_LINE_SIZE constant. It is not depended on
the kernel one.
lilo-22.7 lilo.h:
#define COMMAND_LINE_SIZE 256 /* CL_LENGTH */
lilo-22.7.2 lilo.h:
#define COMMAND_LINE_SIZE 512 /* CL_LENGTH */
So at worse case scenario it passes 256 bytes into the kernel
truncated with null terminated. Best case scenario it passes 512
bytes. Anyway... As long as you don't modify lilo, the kernel can
expect what-ever command-line length it wishes and truncate it to
match its own COMMAND_LINE_SIZE.
Please notice that we modify the kernel so it can accept long command
lines at boot protocol >= 2.02, but we don't modify lilo behavior. So
lilo user will not be able to use the long command line size, until
lilo is modified to support it.
There is also a problem with grub, I've written a patch for it:
https://savannah.gnu.org/bugs/?func=detailitem&item_id=13606
Best Regards,
Alon Bar-Lev.
On Sunday 27 August 2006 21:32, H. Peter Anvin wrote:
> Andi Kleen wrote:
> >
> > 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.
>
> 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.
-Andi
Andi Kleen wrote:
> On Sunday 27 August 2006 21:32, H. Peter Anvin wrote:
>> Andi Kleen wrote:
>>> 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.
>
>> 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
LILO memory usage:
000600 - 001000 BIOS data check area. Okay to overwrite. LILO usage
suppressed with command line "nobd" option. There's also a config
file option to suppress usage.
LILO typically loads at 9000:0000 up to the top of the EBDA. Top of
EBDA is determined by "int 12h." Some BIOS's on add-in cards do not
properly allocate the EBDA. Use LILO Makefile option "BUG_SI_EBDA"
to allocate extra EBDA for the BIOS. If the BIOS data check area is
created at boot time by LILO, then:
> lilo -T ebda
will tell you where LILO is loaded on your system.
--John
At 02:39 PM Sunday 8/27/2006, H. Peter Anvin wrote:
>Andi Kleen wrote:
>>On Sunday 27 August 2006 21:32, H. Peter Anvin wrote:
>>>Andi Kleen wrote:
>>>>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.
>>
>>>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
PGP KeyID: 6781C9C8 (good until 31-Dec-2008)
Keyserver at ldap://keyserver.pgp.com OR http://pgp.mit.edu
LILO links at http://freshmeat.net/projects/lilo
and Help link at http://lilo.go.dyndns.org
H. Peter Anvin wrote:
> 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
@@ -29,6 +29,8 @@
movl $(COMMAND_LINE_SIZE-7), %ecx
# loop through kernel command line one byte at a time
cl_loop:
+ cmpb $0,(%si)
+ jz done_cl
cmpl $EDD_CL_EQUALS, (%si)
jz found_edd_equals
incl %esi
Alon Bar-Lev wrote:
> H. Peter Anvin wrote:
>> 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
> @@ -29,6 +29,8 @@
> movl $(COMMAND_LINE_SIZE-7), %ecx
> # loop through kernel command line one byte at a time
> cl_loop:
> + cmpb $0,(%si)
> + jz done_cl
> cmpl $EDD_CL_EQUALS, (%si)
> jz found_edd_equals
> incl %esi
>
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...)
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 09:34:39.000000000 +0300
@@ -29,7 +29,9 @@
movl $(COMMAND_LINE_SIZE-7), %ecx
# loop through kernel command line one byte at a time
cl_loop:
- cmpl $EDD_CL_EQUALS, (%si)
+ cmpb $0,(%esi)
+ jz done_cl
+ cmpl $EDD_CL_EQUALS, (%esi)
jz found_edd_equals
incl %esi
loop cl_loop
@@ -37,9 +39,9 @@ cl_loop:
found_edd_equals:
# only looking at first two characters after equals
addl $4, %esi
- cmpw $EDD_CL_OFF, (%si) # edd=of
+ cmpw $EDD_CL_OFF, (%esi) # edd=of
jz do_edd_off
- cmpw $EDD_CL_SKIP, (%si) # edd=sk
+ cmpw $EDD_CL_SKIP, (%esi) # edd=sk
jz do_edd_skipmbr
jmp done_cl
do_edd_skipmbr:
Alon Bar-Lev wrote:
>
> 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...)
-hpa
On 8/28/06, H. Peter Anvin <[email protected]> wrote:
> 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?
Best Regards,
Alon Bar-Lev.
Alon Bar-Lev wrote:
> On 8/28/06, H. Peter Anvin <[email protected]> wrote:
>> 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?
>
Probably modifying the whole code to use 16 bits, unless there is a
specific reason not to (Matt?)
-hpa
On Mon, Aug 28, 2006 at 11:28:24AM -0700, H. Peter Anvin wrote:
> Alon Bar-Lev wrote:
> >On 8/28/06, H. Peter Anvin <[email protected]> wrote:
> >>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?
> >
>
> 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?
Thanks,
Matt
--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com
Matt Domsch wrote:
>
> 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.
-hpa
On 8/28/06, Matt Domsch <[email protected]> wrote:
> 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...
Best Regards,
Alon Bar-Lev.
On Mon, Aug 28, 2006 at 12:00:37PM -0700, H. Peter Anvin wrote:
> Matt Domsch wrote:
> >
> >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
--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com
On 8/28/06, Matt Domsch <[email protected]> wrote:
> OK, I'll look at fixing that, and using %esi throughout.
>
> Thanks,
> Matt
>
Something as the attached?
Best Regards,
Alon Bar-Lev.
Alon Bar-Lev wrote:
> On 8/28/06, Matt Domsch <[email protected]> wrote:
>> 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...
>
Bullshit. You're in 16 bit mode, and your segment limits are only 64K
in size, so you HAVE to use a segment:offset type addressing:
Thus, you want to do something like this.
movl cmd_line_ptr, %esi
movl %esi, %eax
shrl $4, %eax
mov %ax, %es
andw $0xf, %si
... and then address it through es:si. Anything else is total, utterly
and completely wrong.
-hpa
Matt Domsch wrote:
> On Mon, Aug 28, 2006 at 12:00:37PM -0700, H. Peter Anvin wrote:
>> Matt Domsch wrote:
>>> 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.
>
NAK on that. "Using %esi throughout" is a red herring, and just will
produce worse code for no gain.
-hpa
Matt Domsch wrote:
> On Mon, Aug 28, 2006 at 12:00:37PM -0700, H. Peter Anvin wrote:
>> Matt Domsch wrote:
>>> 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.
>
There is a lot of weirdness in this code; it's broken in an enormous
amount of ways (sorry, Matt). This comment, for example:
pushl %esi
cmpl $0, %cs:cmd_line_ptr
jz done_cl
movl %cs:(cmd_line_ptr), %esi
# ds:esi has the pointer to the command line now
... doesn't handle the old boot protocol, and doesn't at all deal with
the fact that cmd_line_ptr is an absolute address, and not at all
relative to SETUPSEG, which is the normal value for %ds at this point.
For the old protocol, this is a 16-bit pointer which is relative to
INITSEG (not SETUPSEG), but this code just completely ignores it.
I'll hack up a patch for this.
-hpa
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]>
diff --git a/arch/i386/boot/edd.S b/arch/i386/boot/edd.S
index 4b84ea2..03712a0 100644
--- a/arch/i386/boot/edd.S
+++ b/arch/i386/boot/edd.S
@@ -15,42 +15,90 @@ #include <linux/edd.h>
#include <asm/setup.h>
#if defined(CONFIG_EDD) || defined(CONFIG_EDD_MODULE)
- movb $0, (EDD_MBR_SIG_NR_BUF)
- movb $0, (EDDNR)
-# Check the command line for two options:
+# It is assumed that %ds == INITSEG here
+
+ movb $0, EDD_MBR_SIG_NR_BUF
+ movb $0, EDDNR
+
+# Check the command line for options:
# edd=of disables EDD completely (edd=off)
# edd=sk skips the MBR test (edd=skipmbr)
+# edd=on re-enables EDD (edd=on)
+
pushl %esi
- cmpl $0, %cs:cmd_line_ptr
- jz done_cl
+ movw $edd_mbr_sig_start, %di # Default to edd=on
+
movl %cs:(cmd_line_ptr), %esi
-# ds:esi has the pointer to the command line now
- movl $(COMMAND_LINE_SIZE-7), %ecx
+ 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
+ jmp have_cl_pointer
+
+# 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
+
+# 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
+ andb %al, %al # End of line?
+ jz done_cl
+ cmpl $EDD_CL_EQUALS, %eax
jz found_edd_equals
- incl %esi
- loop cl_loop
- jmp done_cl
+ cmpb $0x20, %al # <= space consider whitespace
+ ja cl_skipword
+ incw %si
+ jnz cl_atspace
+ 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...
+
found_edd_equals:
# only looking at first two characters after equals
- addl $4, %esi
- cmpw $EDD_CL_OFF, (%si) # edd=of
- jz do_edd_off
- cmpw $EDD_CL_SKIP, (%si) # edd=sk
- jz do_edd_skipmbr
- jmp done_cl
+# late overrides early on the command line, so keep going after finding something
+ movw %fs:4(%si), %ax
+ cmpw $EDD_CL_OFF, %ax # edd=of
+ je do_edd_off
+ cmpw $EDD_CL_SKIP, %ax # edd=sk
+ je do_edd_skipmbr
+ cmpw $EDD_CL_ON, %ax # edd=on
+ je do_edd_on
+ jmp cl_skipword
do_edd_skipmbr:
- popl %esi
- jmp edd_start
+ movw $edd_start, %di
+ jmp cl_skipword
do_edd_off:
- popl %esi
- jmp edd_done
+ movw $edd_done, %di
+ jmp cl_skipword
+do_edd_on:
+ movw $edd_mbr_sig_start, %di
+ jmp cl_skipword
+
done_cl:
popl %esi
-
+ jmpw *%di
# Read the first sector of each BIOS disk device and store the 4-byte signature
edd_mbr_sig_start:
diff --git a/include/linux/edd.h b/include/linux/edd.h
index 162512b..b2b3e68 100644
--- a/include/linux/edd.h
+++ b/include/linux/edd.h
@@ -52,6 +52,7 @@ #define EDD_MBR_SIG_NR_BUF 0x1ea /* add
#define EDD_CL_EQUALS 0x3d646465 /* "edd=" */
#define EDD_CL_OFF 0x666f /* "of" for off */
#define EDD_CL_SKIP 0x6b73 /* "sk" for skipmbr */
+#define EDD_CL_ON 0x6e6f /* "on" for on */
#ifndef __ASSEMBLY__
H. Peter Anvin wrote:
>
>
> ------------------------------------------------------------------------
>
> 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]>
>
>
> diff --git a/arch/i386/boot/edd.S b/arch/i386/boot/edd.S
> index 4b84ea2..03712a0 100644
> --- a/arch/i386/boot/edd.S
> +++ b/arch/i386/boot/edd.S
> + 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
> + jmp have_cl_pointer
> +
> +# 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 ?
> +
> +# 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.
> + andb %al, %al # End of line?
> + jz done_cl
> + cmpl $EDD_CL_EQUALS, %eax
> jz found_edd_equals
> - incl %esi
> - loop cl_loop
> - jmp done_cl
> + cmpb $0x20, %al # <= space consider whitespace
> + ja cl_skipword
> + incw %si
> + jnz cl_atspace
You already died with #GP when si was 0xfffd or bigger above, so this ZF test is
probably not quite needed.
> + 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...
> +
> found_edd_equals:
> # only looking at first two characters after equals
> - addl $4, %esi
> - cmpw $EDD_CL_OFF, (%si) # edd=of
> - jz do_edd_off
> - cmpw $EDD_CL_SKIP, (%si) # edd=sk
> - jz do_edd_skipmbr
> - jmp done_cl
> +# late overrides early on the command line, so keep going after finding something
> + movw %fs:4(%si), %ax
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?
Thanks,
Petr Vandrovec
Petr Vandrovec wrote:
>> +
>> +# 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 ?
>
>> +
>> +# 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.
>
With the old protocol, the command line is supposed to fit inside the
64K segment, so I don't think that's an issue. Putting "Hail Mary"
break at 0xfffd isn't a bad idea, though (especially since even if that
is legitimate, we can't fit "edd=" into that one.)
> 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?
Agreed. I'll update the patch shortly.
-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.
EDD issue was fixed recently by H. Peter Anvin, please add this
to mm so more problems may be found.
Signed-off-by: Alon Bar-Lev <[email protected]>
---
diff -ruNp linux-2.6.18-rc4-mm2/include/asm-i386/param.h linux-2.6.18-rc4-mm2.new/include/asm-i386/param.h
--- linux-2.6.18-rc4-mm2/include/asm-i386/param.h 2006-08-25 16:10:56.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/include/asm-i386/param.h 2006-08-26 02:30:52.000000000 +0300
@@ -18,6 +18,5 @@
#endif
#define MAXHOSTNAMELEN 64 /* max length of hostname */
-#define COMMAND_LINE_SIZE 256
#endif
diff -ruNp linux-2.6.18-rc4-mm2/include/asm-i386/setup.h linux-2.6.18-rc4-mm2.new/include/asm-i386/setup.h
--- linux-2.6.18-rc4-mm2/include/asm-i386/setup.h 2006-08-25 16:10:56.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/include/asm-i386/setup.h 2006-08-26 02:30:52.000000000 +0300
@@ -15,7 +15,7 @@
#define MAX_NONPAE_PFN (1 << 20)
#define PARAM_SIZE 4096
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048
#define OLD_CL_MAGIC_ADDR 0x90020
#define OLD_CL_MAGIC 0xA33F
diff -ruNp linux-2.6.18-rc4-mm2/include/asm-ia64/setup.h linux-2.6.18-rc4-mm2.new/include/asm-ia64/setup.h
--- linux-2.6.18-rc4-mm2/include/asm-ia64/setup.h 2006-06-18 04:49:35.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/include/asm-ia64/setup.h 2006-08-26 02:30:52.000000000 +0300
@@ -1,6 +1,6 @@
#ifndef __IA64_SETUP_H
#define __IA64_SETUP_H
-#define COMMAND_LINE_SIZE 512
+#define COMMAND_LINE_SIZE 2048
#endif
diff -ruNp linux-2.6.18-rc4-mm2/include/asm-x86_64/setup.h linux-2.6.18-rc4-mm2.new/include/asm-x86_64/setup.h
--- linux-2.6.18-rc4-mm2/include/asm-x86_64/setup.h 2006-06-18 04:49:35.000000000 +0300
+++ linux-2.6.18-rc4-mm2.new/include/asm-x86_64/setup.h 2006-08-26 02:32:44.000000000 +0300
@@ -1,6 +1,6 @@
#ifndef _x8664_SETUP_H
#define _x8664_SETUP_H
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048
#endif
On Wednesday 30 August 2006 18:49, Alon Bar-Lev wrote:
>
> 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
-Andi
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.
>
> 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..
Best Regards,
Alon Bar-Lev.
On Wednesday 30 August 2006 19:06, Alon Bar-Lev wrote:
> >
> > 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
On Wed, 30 Aug 2006 19:31:14 +0200
Andi Kleen <[email protected]> wrote:
> On Wednesday 30 August 2006 19:06, Alon Bar-Lev wrote:
>
> > >
> > > 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
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.
Then the following files should be modified to use the new kmalloced variable:
./drivers/sbus/char/openprom.c: char *buf = saved_command_line;
./fs/proc/kcore.c: strncpy(prpsinfo.pr_psargs, saved_command_line, ELF_PRARGSZ);
./fs/proc/proc_misc.c: len = sprintf(page, "%s\n", saved_command_line);
Have I got it right?
Best Regards,
Alon Bar-Lev.
Alon Bar-Lev wrote:
> 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.
>
>> 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..
>
The kmalloc approach seems to be The Right Thing.
-hpa
Alon Bar-Lev wrote:
>
> 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
On Wednesday 30 August 2006 20:59, H. Peter Anvin wrote:
> Alon Bar-Lev wrote:
> >
> > 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.
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.
-Andi
Andi Kleen wrote:
> On Wednesday 30 August 2006 20:59, H. Peter Anvin wrote:
>> Alon Bar-Lev wrote:
>>> 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.
>
> 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.
>
That would definitely be the safest option, and probably is the way to go.
-hpa
On Wed, 30 Aug 2006 11:59:40 -0700
"H. Peter Anvin" <[email protected]> wrote:
> Alon Bar-Lev wrote:
> >
> > 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.
Please instruct me how to proceed...
Best Regards,
Alon Bar-Lev.
Alon Bar-Lev wrote:
>
> 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?
>
Submit a patch set, with the common changes in one patch and the
architecture-specific bits broken out per architecture; that way the
individual arch maintainers can look at their piece. Since it's a
simple variable rename, it shouldn't be a big deal.
> 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.
Don't do that. Just change the name of the buffer in head.S.
-hpa
Andi Kleen <[email protected]> wrote:
> On Wednesday 30 August 2006 20:59, H. Peter Anvin wrote:
>> Alon Bar-Lev wrote:
>> > 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.
>
> 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.
If you set the new *saved_cmdline=saved_cmdline_init, the users don't need
to be adjusted at all, and you won't have trouble with code that may be
run before or after kmallocking (if it exists).
--
Ich danke GMX daf?r, die Verwendung meiner Adressen mittels per SPF
verbreiteten L?gen zu sabotieren.
http://david.woodhou.se/why-not-spf.html
Bodo Eggert wrote:
> Andi Kleen <[email protected]> wrote:
>> On Wednesday 30 August 2006 20:59, H. Peter Anvin wrote:
>>> Alon Bar-Lev wrote:
>
>>>> 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.
>> 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.
>
> If you set the new *saved_cmdline=saved_cmdline_init, the users don't need
> to be adjusted at all, and you won't have trouble with code that may be
> run before or after kmallocking (if it exists).
True for C code, but not for assembly.
-hpa