Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Bodo Eggert]

Andi Kleen <ak@suse.de> 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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

Bodo Eggert wrote:
> Andi Kleen <ak@suse.de> 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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

On Wed, 30 Aug 2006 11:59:40 -0700
"H. Peter Anvin" <hpa@zytor.com> 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.

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Andi Kleen]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

Alon Bar-Lev wrote:
> On Wed, 30 Aug 2006 18:56:11 +0200
> Andi Kleen <ak@suse.de> 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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

On Wed, 30 Aug 2006 19:31:14 +0200
Andi Kleen <ak@suse.de> 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.

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Andi Kleen]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

On Wed, 30 Aug 2006 18:56:11 +0200
Andi Kleen <ak@suse.de> 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.

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Andi Kleen]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

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 <alon.barlev@gmail.com>

---

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

Alon Bar-Lev wrote:
> On 8/28/06, Matt Domsch <Matt_Domsch@dell.com> 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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

[-- Attachment #1: Type: text/plain, Size: 191 bytes --]

On 8/28/06, Matt Domsch <Matt_Domsch@dell.com> wrote:
> OK, I'll look at fixing that, and using %esi throughout.
>
> Thanks,
> Matt
>

Something as the attached?

Best Regards,
Alon Bar-Lev.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: edd-esi-null.diff --]
[-- Type: text/x-diff; name="edd-esi-null.diff", Size: 1057 bytes --]

--- 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 23:21:01.000000000 +0300
@@ -22,14 +22,16 @@
 # edd=of  disables EDD completely  (edd=off)
 # edd=sk  skips the MBR test    (edd=skipmbr)
 	pushl	%esi
-    	cmpl	$0, %cs:cmd_line_ptr
-	jz	done_cl
 	movl	%cs:(cmd_line_ptr), %esi
+	andl	%esi, %esi
+	jz	done_cl
 # ds:esi has the pointer to the command line now
-	movl	$(COMMAND_LINE_SIZE-7), %ecx
+	movw	$(COMMAND_LINE_SIZE-7), %cx
 # 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:

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Matt Domsch]

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 & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

On 8/28/06, Matt Domsch <Matt_Domsch@dell.com> 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.

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Matt Domsch]

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 <hpa@zytor.com> 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 & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

Alon Bar-Lev wrote:
> On 8/28/06, H. Peter Anvin <hpa@zytor.com> 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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

On 8/28/06, H. Peter Anvin <hpa@zytor.com> 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.

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

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:

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[John Coffman]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Andi Kleen]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

On 8/27/06, Andi Kleen <ak@suse.de> 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.

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Andi Kleen]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[H. Peter Anvin]

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

Re: [PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Andi Kleen]

Alon Bar-Lev <alon.barlev@gmail.com> 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

[PATCH] THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit (ping)

[Alon Bar-Lev]

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 <alon.barlev@gmail.com>

---

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