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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Wed, Jan 14, 2015 at 11:40:54PM +0530, ashishsangwan2@gmail.com wrote:
> From: Ashish Sangwan <a.sangwan@samsung.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@samsung.com>
> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.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