linux-kernel - Re: [PATCH] device-core: Ensure drvdata = NULL when no driver is bound

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 18 Jul 2013 16:49:18 -0700
From:	"Luis R. Rodriguez" <mcgrof@...not-panic.com>
To:	Hans de Goede <hdegoede@...hat.com>,
	"backports@...r.kernel.org" <backports@...r.kernel.org>,
	Felix Fietkau <nbd@...nwrt.org>,
	Julia Lawall <julia.lawall@...6.fr>,
	linux-wireless <linux-wireless@...r.kernel.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jiri Slaby <jslaby@...e.cz>, Jiri Kosina <jkosina@...e.cz>,
	USB list <linux-usb@...r.kernel.org>
Subject: Re: [PATCH] device-core: Ensure drvdata = NULL when no driver is bound

On Tue, May 22, 2012 at 3:09 PM, Hans de Goede <hdegoede@...hat.com> wrote:
> 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>
> ---
>  drivers/base/dd.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 1b1cbb5..9a1e970 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -283,6 +283,7 @@ probe_failed:
>         devres_release_all(dev);
>         driver_sysfs_remove(dev);
>         dev->driver = NULL;
> +       dev_set_drvdata(dev, NULL);
>
>         if (ret == -EPROBE_DEFER) {
>                 /* Driver requested deferred probing */
> @@ -487,6 +488,7 @@ static void __device_release_driver(struct device *dev)
>                         drv->remove(dev);
>                 devres_release_all(dev);
>                 dev->driver = NULL;
> +               dev_set_drvdata(dev, NULL);
>                 klist_remove(&dev->p->knode_driver);
>                 if (dev->bus)
>                         blocking_notifier_call_chain(&dev->bus->p->bus_notifier,

Just as a heads up for backports:

Because of this patch a huge series of cleanups were made across the
kernel removing redundant setting of the driver data to NULL that was
spread out through the kernel. An example type patch is
785ec305b26ee23e0efb834b5cd0dd24070ba1bf. This is an example type of
collateral evolution that doesn't cause compilation issues if the
upstream code is used but if not addressed could potentially cause an
issue. That is the only way to catch these are by policing /
proactively reviewing kernel changes.

I'll post a patch that backports this to enable usage of the code
as-is upstream on older kernels without having to put the code that
was being removed for each driver specifically shortly.

  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/