linux-kernel - Re: [PATCH] driver core: access dev->driver under dev->mutex in dev

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: <20150114204635.GD31119@kroah.com>
Date:	Wed, 14 Jan 2015 12:46:35 -0800
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	ashishsangwan2@...il.com
Cc:	linux-kernel@...r.kernel.org,
	Ashish Sangwan <a.sangwan@...sung.com>,
	Namjae Jeon <namjae.jeon@...sung.com>
Subject: Re: [PATCH] driver core: access dev->driver under dev->mutex in
 dev_uevent

On Wed, Jan 14, 2015 at 11:40:54PM +0530, ashishsangwan2@...il.com wrote:
> From: Ashish Sangwan <a.sangwan@...sung.com>
> 
> We have hit a race condition while parallely accessing device's uevent
> and rmmoding device's driver.

How are you doing that?  By reading the uevent file and removing the
device?

Removing a kernel module is not a "normal" thing at all, so this type of
"error" is very low on anyone's list.

> In dev_uevent() first dev->driver is checked if
> present which becomes true but before calling add_uevent_var 
> dev->driver is set to NULL and driver is freed in __device_release_driver().
> This results in either NULL pointer derefrence or invalid virtual address
> access. Fix this race by accessing dev->driver under dev->mutex.
> 
> Signed-off-by: Ashish Sangwan <a.sangwan@...sung.com>
> Signed-off-by: Namjae Jeon <namjae.jeon@...sung.com>
> ---
>  drivers/base/core.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 97e2baf..f8e854a 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -326,8 +326,15 @@ static int dev_uevent(struct kset *kset, struct kobject *kobj,
>  	if (dev->type && dev->type->name)
>  		add_uevent_var(env, "DEVTYPE=%s", dev->type->name);
>  
> +	/*
> +	 * dev->driver is set NULL under dev->mutex so it should be checked
> +	 * after acquiring dev->mutex. This prevents a race between 
> +	 * __device_release_driver and dev_uevent.
> +	 */
> +	device_lock(dev);
>  	if (dev->driver)
>  		add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> +	device_unlock(dev);

Always run your patches through checkpatch before sending them out so
you don't get grumpy email responses about how you need to run patches
through checkpatch before sending them out...

thanks,

greg k-h
--
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/