On Fri, Feb 07, 2020 at 02:46:03PM -0500, Steven Rostedt wrote:

On Fri, 7 Feb 2020 10:03:16 -0800
Kees Cook [off-list ref] wrote:

quoted

quoted

+		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.)

This is not the args you are looking for. ;-)

There is a slight bug, but not as bad as you may think it is.
bootconfig (when added to the command line) will look for a json like
file appended to the initrd, and it will parse that. That's what all the
xbc_*() functions do (extended boot commandline). If one of the options
in that json like file is "init", then it will create the
extra_init_args, which will make ilen greater than zero.

The above if statement looks for that ' -- ', and if it doesn't find it
(strcmp() returns NULL when not found) it will than append " -- " to
the boot_command_line. If it is found, then the " -- " is not added. In
either case, the init args found in the json like file in the initrd is
appended to the saved_command_line.

I did say there's a slight bug here. If you have your condition, and
you add init arguments to that json file, it wont properly add the " --
", and the init arguments in that file will be ignored.

Ah, right, it's even more slight, sorry, I had the strstr() in my head
still. So, yes, with an "init" section and a very goofy " -- " present
in a kernel bootparam string value, the appended init args will be
parsed as kernel options.

-- 
Kees Cook