Re: [PATCH 11/12] iio: accel: kionix-kx022a: Prevent memory leak and fix state

[Jonathan Cameron]

On Mon, 10 Aug 2026 10:55:03 +0300
Matti Vaittinen <matti.vaittinen@xxxxxxxxx> wrote:

> From: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
> 
> The driver allocates memory for samples at buffer enable path. If regmap
> operation fails in the kx022a_fifo_enable() at the buffer enable path, the
> allocated memory is never freed. Furthermore, the state information and
> previous hardware configuration(s) aren't undone, potentially leaving
> WMI interrupts and buffers enabled, or driver state flags wrong.
> 
> Free the memory and revert the hardware configuration and state flags on
> error path.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
> Fixes: e7123a4dfcd7 ("iio: accel: kionix-kx022a: Refactor driver and add chip_info structure")
> ---
>  drivers/iio/accel/kionix-kx022a.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index 8a13f78aeab0..49e8b4b943da 100644
> --- a/drivers/iio/accel/kionix-kx022a.c
> +++ b/drivers/iio/accel/kionix-kx022a.c
> @@ -980,26 +980,43 @@ static int kx022a_fifo_enable(struct kx022a_data *data)
>  	guard(mutex)(&data->mutex);
>  	ret = __kx022a_turn_on_off(data, false);
>  	if (ret)
> -		return ret;
> +		goto err_free_out;

If we follow this path we are assuming that turn_on_off hasn't
had any side effects in failing...


>  
>  	/* Update watermark to HW */
>  	ret = kx022a_fifo_set_wmi(data);
>  	if (ret)
> -		return ret;
> +		goto err_free_out;
>  
>  	/* Enable buffer */
>  	ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2,
>  			      KX022A_MASK_BUF_EN);
>  	if (ret)
> -		return ret;
> +		goto err_free_out;
>  
>  	data->state |= KX022A_STATE_FIFO;
>  	ret = regmap_set_bits(data->regmap, data->ien_reg,
>  			      KX022A_MASK_WMI);
>  	if (ret)
> -		return ret;
> +		goto err_wmi_out;
>  
> -	return __kx022a_turn_on_off(data, true);
> +	ret = __kx022a_turn_on_off(data, true);
> +	if (ret)
> +		goto err_on_out;


> +
> +	return ret;
> +
> +err_on_out:
> +	regmap_clear_bits(data->regmap, data->ien_reg,
> +			  KX022A_MASK_WMI);
> +err_wmi_out:
> +	regmap_clear_bits(data->regmap, data->chip_info->buf_cntl2,
> +			  KX022A_MASK_BUF_EN);
> +err_free_out:
> +	kfree(data->fifo_buffer);

This thing is fine here.

> +	data->state &= ~KX022A_STATE_FIFO;

This should also only occur when we have set it in the first place - 
so under err_wmi_out:


> +	__kx022a_turn_on_off(data, true);

So following path above we should not be calling this.  It might
be safe to do so but it isn't logically correct.  It should be a few
lines earlier.

> +
> +	return ret;
>  }
>  
>  static int kx022a_buffer_postenable(struct iio_dev *idev)