linux-kernel - Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support

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]

Date:	Tue, 30 Sep 2014 09:47:35 +0200
From:	"Luis R. Rodriguez" <mcgrof@...e.com>
To:	Tom Gundersen <teg@...m.no>
Cc:	"Luis R. Rodriguez" <mcgrof@...not-panic.com>,
	Michal Hocko <mhocko@...e.cz>,
	Greg KH <gregkh@...uxfoundation.org>,
	Julia Lawall <julia.lawall@...6.fr>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Takashi Iwai <tiwai@...e.de>, Tejun Heo <tj@...nel.org>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Robert Milasan <rmilasan@...e.com>, werner@...e.com,
	Oleg Nesterov <oleg@...hat.com>, hare <hare@...e.com>,
	Benjamin Poirier <bpoirier@...e.de>,
	Santosh Rastapur <santosh@...lsio.com>, pmladek@...e.cz,
	dbueso@...e.com, LKML <linux-kernel@...r.kernel.org>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Joseph Salisbury <joseph.salisbury@...onical.com>,
	Kay Sievers <kay@...y.org>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Tim Gardner <tim.gardner@...onical.com>,
	Pierre Fersing <pierre-fersing@...rref.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Nagalakshmi Nandigama <nagalakshmi.nandigama@...gotech.com>,
	Praveen Krishnamoorthy <praveen.krishnamoorthy@...gotech.com>,
	Sreekanth Reddy <sreekanth.reddy@...gotech.com>,
	Abhijit Mahajan <abhijit.mahajan@...gotech.com>,
	Casey Leedom <leedom@...lsio.com>,
	Hariprasad S <hariprasad@...lsio.com>,
	"mpt-fusionlinux.pdl" <MPT-FusionLinux.pdl@...gotech.com>,
	Linux SCSI List <linux-scsi@...r.kernel.org>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe
	support

On Tue, Sep 30, 2014 at 04:27:51AM +0200, Luis R. Rodriguez wrote:
> On Sun, Sep 28, 2014 at 07:07:24PM +0200, Tom Gundersen wrote:
> > On Fri, Sep 26, 2014 at 11:57 PM, Luis R. Rodriguez
> > <mcgrof@...not-panic.com> wrote:
> > > From: "Luis R. Rodriguez" <mcgrof@...e.com>
> > > 0) Not all drivers are killed, the signal is just sent and
> > >    the kill will only be acted upoon if the driver you loaded
> > >    happens to have some code path that either uses kthreads (which
> > >    as of 786235ee are now killable), or uses some code which checks for
> > >    fatal_signal_pending() on the kernel somewhere -- i.e: pci_read_vpd().
> > 
> > Shouldn't this be seen as something to be fixed in the kernel?
> 
> That's a great question. In practice now after CVE-2012-4398 and its series of
> patches added which enabled OOM to kill things followed by 786235ee to also
> handle OOM on kthreads it seems imperative we strive towards this, in practive
> however if you're getting OOMs on boot you have far more serious issue to be
> concerned over than handling CVE-2012-4398. Another issue is that even if we
> wanted to address this a critical right now on module loading driver error
> paths tend to be pretty buggy and we'd probably end up causing more issues than
> fixing anything if the sigkill that triggered this was an arbitrary timeout,
> specially if the timeout is not properly justified.

<-- snip -->

> So extending the kill onto more drivers *because* of the timeout is probably
> not a good reason as it would probably create more issue than fix anything
> right now.

A bit more on this. Tejun had added devres while trying to convert libata to
use iomap but in that process also help address buggy failure paths on drivers [0].
Even with devres in place and devm functions being available they actually
haven't been popularized until recent kernels [1]. There is even further
research on precicely these sorts of errors, such as "Hector: Detecting
Resource-Release Omission Faults in error-handling code for systems software" [2]
but unfortunately there is no data over time. Another paper is "An approach to
improving the structure of error-handling code in the Linux kernel" [3] which
tries to address moving error handling code in the middle of the function to gotos
to shared code at the end of the function...

So we have buggy error paths on drivers and trusting them unfortunately isn't
a good idea at this point. They should be fixed but saying we should equally
kill all drivers right now would likley introduce more issues than anything.

[0] http://lwn.net/Articles/215861/
[1] http://www.slideshare.net/ennael/kernel-recipes-2013?qid=f0888b85-377b-4b29-95c3-f4e59822f5b3&v=default&b=&from_search=6
    See slide 6 on graph usage of devm functions over time
[2] http://coccinelle.lip6.fr/papers/dsn2013.pdf
[3] http://coccinelle.lip6.fr/papers/lctes11.pdf

  Luis
--
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/