linux-kernel - Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism

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: <20111011134753.2751aeb1.akpm@linux-foundation.org>
Date:	Tue, 11 Oct 2011 13:47:53 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Greg Kroah-Hartman <greg@...ah.com>,
	Dilan Lee <dilee@...dia.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Manjunath GKondaiah <manjunath.gkondaiah@...aro.org>,
	Arnd Bergmann <arnd@...db.de>
Subject: Re: [RFC PATCH v3] drivercore: Add driver probe deferral mechanism

On Thu, 22 Sep 2011 12:51:23 -0600
Grant Likely <grant.likely@...retlab.ca> wrote:

> Allow drivers to report at probe time that they cannot get all the resources
> required by the device, and should be retried at a later time.
> 
> This should completely solve the problem of getting devices
> initialized in the right order.  Right now this is mostly handled by
> mucking about with initcall ordering which is a complete hack, and
> doesn't even remotely handle the case where device drivers are in
> modules.  This approach completely sidesteps the issues by allowing
> driver registration to occur in any order, and any driver can request
> to be retried after a few more other drivers get probed.

What happens is there is a circular dependency, or if a driver's
preconditions are never met?  AFAICT the code keeps running the probe
function for ever.

If so: bad.  The kernel should detect such situations, should
exhaustively report them and if possible, fix them up and struggle
onwards.

>
> ...
>
> +	 * This bit is tricky.  We want to process every device in the
> +	 * deferred list, but devices can be removed from the list at any
> +	 * time while inside this for-each loop.  There are two things that
> +	 * need to be protected against:
> +	 * - if the device is removed from the deferred_probe_list, then we
> +	 *   loose our place in the loop.  Since any device can be removed

s/loose/lose/

> +	 *   asynchronously, list_for_each_entry_safe() wouldn't make things
> +	 *   much better.  Simplest solution is to restart walking the list
> +	 *   whenever the current device gets removed.  Not the most efficient,
> +	 *   but is simple to implement and easy to audit for correctness.
> +	 * - if the device is unregistered, and freed, then there is a risk
> +	 *   of a null pointer dereference.  This code uses get/put_device()
> +	 *   to ensure the device cannot disappear from under our feet.
> +	 */
>
> ...
>
> +		/* Drop the mutex while probing each device; the probe path
> +		 * may manipulate the deferred list */

Please don't invent new coding styles.  Like this:

		/*
		 * Drop the mutex while probing each device; the probe path
		 * may manipulate the deferred list
		 */

(entire patch)

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