Hi,
On 2018년 08월 30일 19:02, [email protected] wrote:
> From: Vincent Donnefort <[email protected]>
>
> device_release() is freeing the resources before calling the device
> specific release callback which is, in the case of devfreq, stopping
> the governor.
>
> It is a problem as some governors are using the device resources. e.g.
> simpleondemand which is using the devfreq deferrable monitoring work. If it
> is not stopped before the resources are freed, it might lead to a use after
> free.
>
> Signed-off-by: Vincent Donnefort <[email protected]>
> Reviewed-by: John Einar Reitan <[email protected]>
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 4c49bb1..4e43830 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -534,10 +534,6 @@ static void devfreq_dev_release(struct device *dev)
> list_del(&devfreq->node);
> mutex_unlock(&devfreq_list_lock);
>
> - if (devfreq->governor)
> - devfreq->governor->event_handler(devfreq,
> - DEVFREQ_GOV_STOP, NULL);
> -
> if (devfreq->profile->exit)
> devfreq->profile->exit(devfreq->dev.parent);
>
> @@ -672,7 +668,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> list_del(&devfreq->node);
> mutex_unlock(&devfreq_list_lock);
>
> - device_unregister(&devfreq->dev);
> + devfreq_remove_device(devfreq);
> devfreq = NULL;
> err_dev:
> if (devfreq)
> @@ -693,6 +689,9 @@ int devfreq_remove_device(struct devfreq *devfreq)
> if (!devfreq)
> return -EINVAL;
>
> + if (devfreq->governor)
> + devfreq->governor->event_handler(devfreq,
> + DEVFREQ_GOV_STOP, NULL);
> device_unregister(&devfreq->dev);
>
> return 0;
>
As description of this patch, if devfreq_wq is executed and then execute
the 'devfreq->governor->get_target_freq' between step1 and step2
after already freed the 'dev' related resource, it might happen the problem
because the registered callback of get_target_freq requires the 'dev' resource.
device_unregister(dev)
step 1. device_del(dev)
<- if devfreq_wq is executed
step 2. put_device(dev)
device_release()
devfreq_dev_release()
stop the governor for specific devfreq instance
It looks good to me. Stop the governor before calling device_unregister().
Reviewed-by: Chanwoo Choi <[email protected]>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
