linux-kernel - Re: [PATCH] backports: backport drvdata = NULL core driver fixes

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: <Pine.LNX.4.44L0.1307191012330.1055-100000@iolanthe.rowland.org>
Date:	Fri, 19 Jul 2013 10:17:06 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	"Luis R. Rodriguez" <mcgrof@...not-panic.com>
cc:	backports@...r.kernel.org, Hans de Goede <hdegoede@...hat.com>,
	Julia Lawall <julia.lawall@...6.fr>,
	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	Jiri Slaby <jslaby@...e.cz>, Jiri Kosina <jkosina@...e.cz>,
	Felix Fietkau <nbd@...nwrt.org>
Subject: Re: [PATCH] backports: backport drvdata = NULL core driver fixes

On Thu, 18 Jul 2013, Luis R. Rodriguez wrote:

> From: "Luis R. Rodriguez" <mcgrof@...not-panic.com>
> 
> The Linux kernel had tons of code which at times cleared the
> drvdata upon probe failure or release. There are however a bunch
> of drivers that didn't clear this.
> 
> Commit 0998d063 implmented clearing this upon device_release_driver()
> and dealt with probe failure on driver_probe_device(). After this the
> kernel was cleaned up separately with *tons* of patches to remove all
> these driver specific settings given that the clearing is now done
> internally by the device core.
> 
> Instead of ifdef'ing code back in for older code where it was properly
> in place backport this by piggy backing the new required code upon the
> calls used in place. There is a small race here upon device_release_driver()
> but we can live with that theoretical race.
> 
> Due to the way we hack this backport we can't use a separate namespace
> as we have with other symbols.
> 
> mcgrof@...jol ~/linux-stable (git::master)$ git describe --contains \
> 	0998d0631001288a5974afc0b2a5f568bcdecb4d
> v3.6-rc1~99^2~14^2~17
> 
> commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
> Author: Hans de Goede <hdegoede@...hat.com>
> Date:   Wed May 23 00:09:34 2012 +0200
> 
>     device-core: Ensure drvdata = NULL when no driver is bound
> 
>     1) drvdata is for a driver to store a pointer to driver specific data
>     2) If no driver is bound, there is no driver specific data associated with
>        the device
>     3) Thus logically drvdata should be NULL if no driver is bound.
> 
>     But many drivers don't clear drvdata on device_release, or set drvdata
>     early on in probe and leave it set on probe error. Both of which results
>     in a dangling pointer in drvdata.
> 
>     This patch enforce for drvdata to be NULL after device_release or on probe
>     failure.
> 
>     Signed-off-by: Hans de Goede <hdegoede@...hat.com>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

This is not a very good idea.  Although setting drvdata to NULL allowed
a lot of code to be removed, it also exposed a bunch of hidden bugs --
drivers were using the drvdata value even after their remove function
returned.

Several commits have been applied to fix those bugs.  Finding and
backporting them and their dependencies will not be easy.

I suggest this patch not be applied.

Alan Stern

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