On Sun, Mar 10, 2019 at 03:32:46PM -0400, Sven Van Asbroeck wrote:
> This driver's remove path never explicitly cancels the
> delayed work. So it is possible for the delayed work to
> run after the core has freed the private structure
> (struct envelope). This is a potential use-after-free.
>
> Fix by adding a devm_add_action callback to the remove
> path, called right after iio_device_unregister(), which
> explicitly cancels the delayed work.
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Sven Van Asbroeck <TheSven73@xxxxxxxxx>
> ---
> drivers/iio/adc/envelope-detector.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/iio/adc/envelope-detector.c b/drivers/iio/adc/envelope-detector.c
> index 2f2b563c1162..2f1c78b3ff44 100644
> --- a/drivers/iio/adc/envelope-detector.c
> +++ b/drivers/iio/adc/envelope-detector.c
> @@ -321,6 +321,14 @@ static const struct iio_info envelope_detector_info = {
> .read_raw = &envelope_detector_read_raw,
> };
>
> +static void envelope_detector_stop_work(void *data)
> +{
> + struct iio_dev *indio_dev = data;
> + struct envelope *env = iio_priv(indio_dev);
> +
> + cancel_delayed_work_sync(&env->comp_timeout);
> +}
> +
> static int envelope_detector_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -395,6 +403,10 @@ static int envelope_detector_probe(struct platform_device *pdev)
> return ret;
> }
>
> + ret = devm_add_action(dev, envelope_detector_stop_work, indio_dev);
> + if (ret)
> + return ret;
Just a random thought. Wouldn't devm_add_action_or_reset() be a better fit?
In case adding action results in failure we will not get the chance to cancel
work.
> +
> return devm_iio_device_register(dev, indio_dev);
> }
>
> --
> 2.17.1
>
