On Fri, Feb 07, 2020 at 10:03:16AM -0800, Kees Cook wrote:
> > +
> > + if (ilen) {
> > + /*
> > + * Append supplemental init boot args to saved_command_line
> > + * so that user can check what command line options passed
> > + * to init.
> > + */
> > + len = strlen(saved_command_line);
> > + if (!strstr(boot_command_line, " -- ")) {
> > + strcpy(saved_command_line + len, " -- ");
> > + len += 4;
> > + } else
> > + saved_command_line[len++] = ' ';
> > +
> > + strcpy(saved_command_line + len, extra_init_args);
> > + }
>
> This isn't safe because it will destroy any argument with " -- " in
> quotes and anything after it. For example, booting with:
>
> thing=on acpi_osi="! -- " other=setting
>
> will wreck acpi_osi's value and potentially overwrite "other=settings",
> etc.
>
> (Yes, this seems very unlikely, but you can't treat " -- " as special,
> the command line string must be correct parsed for double quotes, as
> parse_args() does.)
>
I think it won't overwrite anything, it will just leave out the " -- "
that should have been added?
I wonder if this is necessary, though -- since commit b88c50ac304a ("log
arguments and environment passed to init") the init arguments will be in
the kernel log anyway.
