[PATCH][2.6-mm] early_param console_setup clobbers commandline

[PATCH][2.6-mm] early_param console_setup clobbers commandline

[Zwane Mwaikambo]

console_setup clobbers the argument it's passed, resulting in
the commandline being chopped (it inserts a '\0'). A sample command line
would be;

ro root=/dev/hda1 profile=2 debug console=tty0 console=ttyS0,38400 hugepages=20 nmi_watchdog=2

would end up like;

ro root=/dev/hda1 profile=2 debug console=tty0 console=ttyS0

I believe this may be the only setup call which clobbers the argument, so
how about we pass a copy (i wasn't sure about what would be a decent
argument length so i just went with COMMAND_LINE_SIZE (ugly, yes)?
Something like the following tested patch;

Index: linux-2.6.5-rc3-mm4/init/main.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.5-rc3-mm4/init/main.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 main.c
--- linux-2.6.5-rc3-mm4/init/main.c	2 Apr 2004 03:55:28 -0000	1.1.1.1
+++ linux-2.6.5-rc3-mm4/init/main.c	3 Apr 2004 01:21:34 -0000
@@ -410,6 +410,7 @@ void __init parse_early_options(char **c
 	char *from = *cmdline_p;	/* Original. */
 	char c = ' ', *to = tmp_command_line;	/* Parsed. */
 	int len = 0;
+	char arg[COMMAND_LINE_SIZE];

 	/* Save it, if we need to. */
 	if (*cmdline_p != saved_command_line)
@@ -431,7 +432,10 @@ void __init parse_early_options(char **c
 					/* Find the end of this arg, and
 					 * no spaces allowed in an arg. */
 					after = strchr(from, ' ');
-					if (p->fn(from) < 0) {
+					if (after)
+						*after = '\0';
+					strncpy(arg, from, sizeof(arg)-1);
+					if (p->fn(arg) < 0) {
 						from = before;
 						break;
 					}

Re: [PATCH][2.6-mm] early_param console_setup clobbers commandline

[Tom Rini]

On Fri, Apr 02, 2004 at 08:32:10PM -0500, Zwane Mwaikambo wrote:
> console_setup clobbers the argument it's passed, resulting in
> the commandline being chopped (it inserts a '\0'). A sample command line
> would be;
> 
> ro root=/dev/hda1 profile=2 debug console=tty0 console=ttyS0,38400 hugepages=20 nmi_watchdog=2
> 
> would end up like;
> 
> ro root=/dev/hda1 profile=2 debug console=tty0 console=ttyS0

This shouldn't be a problem 'tho, since we don't allow for spaces in
args, and we do find where the next space is, and ensure it's still a
space after the call (because console can splice up the command line,
but we'd skip over those bits anyhow).

> I believe this may be the only setup call which clobbers the argument, so
> how about we pass a copy (i wasn't sure about what would be a decent
> argument length so i just went with COMMAND_LINE_SIZE (ugly, yes)?
> Something like the following tested patch;

pci= will clobber as well, which is why I thought I asked Andrew to drop
that part of the i386 patch (but perhaps I forgot, and with Rusty's
patch, it becomes a non-issue again).

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [PATCH][2.6-mm] early_param console_setup clobbers commandline

[Zwane Mwaikambo]

On Fri, 2 Apr 2004, Tom Rini wrote:

> This shouldn't be a problem 'tho, since we don't allow for spaces in
> args, and we do find where the next space is, and ensure it's still a
> space after the call (because console can splice up the command line,
> but we'd skip over those bits anyhow).

Another new thing is that all setup functions get called with their
parameter and any other trailing arguments. So console_setup sees;

tty0 console=ttyS0,38400 hugepages=20 nmi_watchdog=2

That's different enough to cause potential problems in future.

> pci= will clobber as well, which is why I thought I asked Andrew to drop
> that part of the i386 patch (but perhaps I forgot, and with Rusty's
> patch, it becomes a non-issue again).

What is the patch name for Rusty's patch?

Re: [PATCH][2.6-mm] early_param console_setup clobbers commandline

[Tom Rini]

On Sat, Apr 03, 2004 at 10:40:01AM -0500, Zwane Mwaikambo wrote:

> On Fri, 2 Apr 2004, Tom Rini wrote:
> 
> > This shouldn't be a problem 'tho, since we don't allow for spaces in
> > args, and we do find where the next space is, and ensure it's still a
> > space after the call (because console can splice up the command line,
> > but we'd skip over those bits anyhow).
> 
> Another new thing is that all setup functions get called with their
> parameter and any other trailing arguments. So console_setup sees;
> 
> tty0 console=ttyS0,38400 hugepages=20 nmi_watchdog=2
> 
> That's different enough to cause potential problems in future.

I _thought_ I had put in bits to make it only pass along the argument,
but perhaps that got lost in my testing/retesting.

> > pci= will clobber as well, which is why I thought I asked Andrew to drop
> > that part of the i386 patch (but perhaps I forgot, and with Rusty's
> > patch, it becomes a non-issue again).
> 
> What is the patch name for Rusty's patch?

I don't know, since I think once he got it working he forgot to CC lkml.
But I certainly hope it's in the next -mm since it replaced the
parse_early_options parsing code with parse_args, so all of the stupid
things my re-implementation got wrong, it doesn't.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [PATCH][2.6-mm] early_param console_setup clobbers commandline

[Andrew Morton]

Tom Rini <trini@kernel.crashing.org> wrote:
>
> > What is the patch name for Rusty's patch?
> 
>  I don't know, since I think once he got it working he forgot to CC lkml.
>  But I certainly hope it's in the next -mm since it replaced the
>  parse_early_options parsing code with parse_args, so all of the stupid
>  things my re-implementation got wrong, it doesn't.

Yup, I added Rusty's patch.  It kinda wrecked everything and needs to be
split up and sprintkled across the various earlier patches, but I haven't
done that yet.

I probably won't prepare another -mm until tomorrow though.

Re: [PATCH][2.6-mm] early_param console_setup clobbers commandline

[Zwane Mwaikambo]

On Sat, 3 Apr 2004, Andrew Morton wrote:

> Tom Rini <trini@kernel.crashing.org> wrote:
> >
> > > What is the patch name for Rusty's patch?
> >
> >  I don't know, since I think once he got it working he forgot to CC lkml.
> >  But I certainly hope it's in the next -mm since it replaced the
> >  parse_early_options parsing code with parse_args, so all of the stupid
> >  things my re-implementation got wrong, it doesn't.
>
> Yup, I added Rusty's patch.  It kinda wrecked everything and needs to be
> split up and sprintkled across the various earlier patches, but I haven't
> done that yet.
>
> I probably won't prepare another -mm until tomorrow though.

Do you still want the console_setup patch?

Re: [PATCH][2.6-mm] early_param console_setup clobbers commandline

[Andrew Morton]

Zwane Mwaikambo <zwane@linuxpower.ca> wrote:
>
> On Sat, 3 Apr 2004, Andrew Morton wrote:
> 
> > Tom Rini <trini@kernel.crashing.org> wrote:
> > >
> > > > What is the patch name for Rusty's patch?
> > >
> > >  I don't know, since I think once he got it working he forgot to CC lkml.
> > >  But I certainly hope it's in the next -mm since it replaced the
> > >  parse_early_options parsing code with parse_args, so all of the stupid
> > >  things my re-implementation got wrong, it doesn't.
> >
> > Yup, I added Rusty's patch.  It kinda wrecked everything and needs to be
> > split up and sprintkled across the various earlier patches, but I haven't
> > done that yet.
> >
> > I probably won't prepare another -mm until tomorrow though.
> 
> Do you still want the console_setup patch?

If it is still relevant and if the bug which it fixes is still present, yes
please.  I plonked my current rollup against rc3 at
http://www.zip.com.au/~akpm/linux/patches/stuff/z.gz.  I think it compiles ;)

Re: [PATCH][2.6-mm] early_param console_setup clobbers commandline

[Zwane Mwaikambo]

On Sat, 3 Apr 2004, Andrew Morton wrote:

> If it is still relevant and if the bug which it fixes is still present, yes
> please.  I plonked my current rollup against rc3 at
> http://www.zip.com.au/~akpm/linux/patches/stuff/z.gz.  I think it compiles ;)

It's taken care of in that rollup.

Thanks