Re: [PATCH] iio: adc: ad_sigma_delta: fix use-after-free on unbind

["Nuno Sá" <nuno.sa@analog.com>]

On Wed, Sep 23, 2026 at 09:48:07AM +0000, Fan Wu wrote:
> ad_sd_buffer_postenable() allocates sigma_delta->samples_buf with
> devm_krealloc() at runtime, so its devres entry sits after all
> probe-time entries of the driver. devm resources are released in
> reverse allocation order, which means unbind frees samples_buf before
> iio_device_unregister() disables the buffers and detaches the trigger
> pollfunc. The data ready IRQ is still enabled at that point, so
> ad_sd_trigger_handler() can still run and memcpy() incoming samples
> into the freed samples_buf.
> 
> Fix this by preallocating the buffer in
> devm_ad_sd_setup_buffer_and_trigger(), before the triggered buffer and
> the IRQ are set up, so it is freed only after iio_device_unregister()
> has drained the trigger handler via free_irq(). Size it for the worst
> case of all sequencer slots being active; ad_sd_validate_scan_mask()
> already caps the number of active channels at num_slots.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: 8bea9af887de ("iio: adc: ad_sigma_delta: Add sequencer support")
> Cc: stable@vger.kernel.org
> Co-developed-by: Song Li <songl@zju.edu.cn>
> Signed-off-by: Song Li <songl@zju.edu.cn>
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---

Makes sense to me! One minor nit Jonathan might be ale to tweak when
applying. With that:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/ad_sigma_delta.c | 32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
> index 1b41029..4f982c6 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -498,7 +498,6 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
>  	const struct iio_scan_type *scan_type = &indio_dev->channels[0].scan_type;
>  	struct spi_transfer *xfer = sigma_delta->sample_xfer;
>  	unsigned int i, slot, channel;
> -	u8 *samples_buf;
>  	int ret;
>  
>  	if (sigma_delta->num_slots == 1) {
> @@ -530,7 +529,7 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
>  		xfer[1].bits_per_word = scan_type->realbits;
>  		xfer[1].len = spi_bpw_to_bytes(scan_type->realbits);
>  	} else {
> -		unsigned int samples_buf_size, scan_size;
> +		unsigned int scan_size;
>  
>  		if (sigma_delta->active_slots > 1) {
>  			ret = ad_sigma_delta_append_status(sigma_delta, true);
> @@ -538,17 +537,6 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
>  				return ret;
>  		}
>  
> -		samples_buf_size =
> -			ALIGN(slot * BITS_TO_BYTES(scan_type->storagebits),
> -			      sizeof(s64));
> -		samples_buf_size += sizeof(s64);
> -		samples_buf = devm_krealloc(&sigma_delta->spi->dev,
> -					    sigma_delta->samples_buf,
> -					    samples_buf_size, GFP_KERNEL);
> -		if (!samples_buf)
> -			return -ENOMEM;
> -
> -		sigma_delta->samples_buf = samples_buf;
>  		scan_size = BITS_TO_BYTES(scan_type->realbits + scan_type->shift);
>  		/* For 24-bit data, there is an extra byte of padding. */
>  		xfer[1].rx_buf = &sigma_delta->rx_buf[scan_size == 3 ? 1 : 0];
> @@ -855,6 +843,24 @@ int devm_ad_sd_setup_buffer_and_trigger(struct device *dev, struct iio_dev *indi
>  
>  		indio_dev->setup_ops = &ad_sd_buffer_setup_ops;
>  	} else {
> +		const struct iio_scan_type *scan_type =
> +			&indio_dev->channels[0].scan_type;
> +		unsigned int samples_buf_size;
> +
> +		/*
> +		 * Worst-case size: all sequencer slots can be active, capped
> +		 * at num_slots by ad_sd_validate_scan_mask().
> +		 */
> +		samples_buf_size =
> +			ALIGN(sigma_delta->num_slots *
> +			      BITS_TO_BYTES(scan_type->storagebits),
> +			      sizeof(s64));
> +		samples_buf_size += sizeof(s64);
> +		sigma_delta->samples_buf =
> +			devm_kzalloc(dev, samples_buf_size, GFP_KERNEL);

Why the line break for devm_kzalloc(). Keep it in the same line please.
Tbh same for for the ALIGN() call but at least that one is coherent with
the original code.

- Nuno Sá

> +		if (!sigma_delta->samples_buf)
> +			return -ENOMEM;
> +
>  		ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
>  						      &iio_pollfunc_store_time,
>  						      &ad_sd_trigger_handler,
>