Re: [PATCH] init: fix in-place parameter modification regression

[Rusty Russell <rusty@rustcorp.com.au>]

Krzysztof Mazur <krzysiek@podlesie.net> writes:
> On Mon, Oct 14, 2013 at 12:34:02PM +0100, Pawel Moll wrote:
>> So, assuming that it is actually legal to modify static_command_line in
>> __setup()-s (and I must say I have rather mixed feelings about it ;-),
>
> I also have mixed feelings about that, but the parse_args() already
> does that, because some characters are replaced with '\0' to split
> command line into separate strings. The ubd driver does the same
> to split parameter into two strings.

Back when there was almost no parameter parsing support, everyone got
used to keeping pointers into the original.  Making everyone kstrdup()
seems like gratuitous churn which is likely to make more bugs.

Your fix means __setup() gets treated specially, in that only it can
mangle the command line.  That makes sense.  But it introduces another
regression: normal parsing functions can't keep pointers, since that's
now __initdata.

There are two possible solutions:
(1) Audit all __setup to make sure they copy if they want to mangle.
    There are about 750 of them, but many are trivial.
(2) alloc_bootmem() a third commandline for parsing.

Now, many functions of form __setup("XXX=") should be turned into
module_param anyway.

I suggest we do (2) for the moment, and start sweeping through cleaning
up __setup() in the longer term.

Cheers,
Rusty.