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;
}
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/
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?
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/
Tom Rini <[email protected]> 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.
On Sat, 3 Apr 2004, Andrew Morton wrote:
> Tom Rini <[email protected]> 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?
Zwane Mwaikambo <[email protected]> wrote:
>
> On Sat, 3 Apr 2004, Andrew Morton wrote:
>
> > Tom Rini <[email protected]> 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 ;)
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