linux-kernel - Re: [PATCH 2/2] serial: pl011: Move uart_register

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: <20140214000717.GG30257@n2100.arm.linux.org.uk>
Date:	Fri, 14 Feb 2014 00:07:17 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	Tushar Behera <tushar.behera@...aro.org>,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org, jslaby@...e.cz,
	ben.dooks@...ethink.co.uk, broonie@...nel.org
Subject: Re: [PATCH 2/2] serial: pl011: Move uart_register_driver call to
	device probe

On Thu, Feb 13, 2014 at 03:26:06PM -0800, Greg KH wrote:
> On Thu, Feb 13, 2014 at 06:42:49PM +0000, Russell King - ARM Linux wrote:
> > We went through this before, and I stated the paths, and no one disagreed
> > with that.
> > 
> > It /is/ racy.
> 
> Ok, I just went and looked at the uart driver register path, and I don't
> see the race (note, if there is one, it's there today, regardless of
> this patch).

The race isn't the uart code, it's the driver model.

Consider what happens when this happens:

* Two pl011 devices get registered at the same time by two different
  threads.

* Both devices have a lock taken on the _device_ itself before matching
  against the driver.

* Both devices get matched to the same driver.

* Both devices are passed into the driver's probe function.

* Both check uart_reg.state, both call uart_register_driver() on that
  at the same time, which results in two allocations inside
  uart_register_driver(), one gets overwritten...

So, the /only/ thing which stops this happening is that the devices
are generally available before the driver is registered, and driver
registration results in devices being probed serially.  Moreover, both
attempt to call tty_register_driver()... one succeeds, the other fails.

However, what about the userspace bind/unbind methods.  Yes, userspace
can ask the driver core to unbind devices from a driver or bind - and
again, there's no per-driver locking here.  So, if you can trigger two
concurrent binds from userspace, you hit the same race as above.

So, if you want to accept these patches, go ahead, introduce races, but
personally I'd recommend plugging these races.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/