linux-kernel - Re: [PATCH 2/3] Make static usermode helper binaries constant

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: <20170117071347.GA7296@kroah.com>
Date:   Tue, 17 Jan 2017 08:13:47 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "J. Bruce Fields" <bfields@...ldses.org>
Cc:     kernel-hardening@...ts.openwall.com, linux-kernel@...r.kernel.org,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Thomas Sailer <t.sailer@...mni.ethz.ch>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Johan Hovold <johan@...nel.org>, Alex Elder <elder@...nel.org>,
        Jeff Layton <jlayton@...chiereds.net>,
        David Howells <dhowells@...hat.com>, NeilBrown <neilb@...e.com>
Subject: Re: [PATCH 2/3] Make static usermode helper binaries constant

On Mon, Jan 16, 2017 at 04:25:55PM -0500, J. Bruce Fields wrote:
> On Mon, Jan 16, 2017 at 05:50:31PM +0100, Greg KH wrote:
> > From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > 
> > There are a number of usermode helper binaries that are "hard coded" in
> > the kernel today, so mark them as "const" to make it harder for someone
> > to change where the variables point to.
> > 
> ...
> > --- a/drivers/pnp/pnpbios/core.c
> > +++ b/drivers/pnp/pnpbios/core.c
> > @@ -98,6 +98,7 @@ static struct completion unload_sem;
> >   */
> >  static int pnp_dock_event(int dock, struct pnp_docking_station_info *info)
> >  {
> > +	static char const sbin_pnpbios[] = "/sbin/pnpbios";
> >  	char *argv[3], **envp, *buf, *scratch;
> >  	int i = 0, value;
> >  
> > @@ -112,7 +113,7 @@ static int pnp_dock_event(int dock, struct pnp_docking_station_info *info)
> >  	 * integrated into the driver core and use the usual infrastructure
> >  	 * like sysfs and uevents
> >  	 */
> > -	argv[0] = "/sbin/pnpbios";
> > +	argv[0] = (char *)sbin_pnpbios;
> 
> So here and elsewhere, can attackers write to argv[0] instead of to the
> memory where the string lives?

Yes, they could, it would be a very "tight" race to do that (have to
write after the assignment and before the call_usermodehelper_exec()
runs).  However, the kernel does not run argv[0], it just passes it to
the binary you specify in path, so for this example, the correct program
would still be run by the kernel.

But, if you do worry about this type of attack, then enable the option I
created in patch 3/3 here, which will funnel all calls into a single
userspace binary where you can then filter on argv[0] to see if you want
to run the binary or not to prevent this type of attack.

> Apologies if I'm rehashing earlier discussion, I did a quick search of
> archives but could easily have missed something.

No problem at all, hopefully I've explained it better now.

thanks,

greg k-h