linux-kernel - Re: [PATCH v6 08/22] bootconfig: init: Allow admin to use bootconfig for init command line

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202002071640.49BFDA2D1A@keescook>
Date:   Fri, 7 Feb 2020 16:44:49 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Frank Rowand <frowand.list@...il.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Tim Bird <Tim.Bird@...y.com>, Jiri Olsa <jolsa@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Tom Zanussi <tom.zanussi@...ux.intel.com>,
        Rob Herring <robh+dt@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Jonathan Corbet <corbet@....net>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-doc@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 08/22] bootconfig: init: Allow admin to use bootconfig
 for init command line

On Fri, Feb 07, 2020 at 02:46:03PM -0500, Steven Rostedt wrote:
> On Fri, 7 Feb 2020 10:03:16 -0800
> Kees Cook <keescook@...omium.org> wrote:
> > > +		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