Re: [PATCH] iio: envelope-detector: fix use-after-free on device remove

[Tomasz Duszynski]

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
>