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

On 12/08/2026 14:47, Mehdi Djait wrote:
> Hi Matti,
> 
> Thank you for the patch!
> 
> On Mon, Aug 10, 2026 at 10:55:03AM +0300, Matti Vaittinen wrote:
>> From: Matti Vaittinen <[email protected]>
>>
>> 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.
> 
> Reviewed-by: Mehdi Djait <[email protected]>
> 
>> Signed-off-by: Matti Vaittinen <[email protected]>
>> 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)
> 
> Do you think this error handling should also be done for kx022a_fifo_disable():
> in case __kx022a_turn_on_off() or one of the regmap_clear_bits() fails before
> we free the fifo_buffer ?
Good question. I am not really sure what IIO thinks if disable returns 
an error. Should the driver be in a state where it can handle re-try of 
a disable (Eg, a new call)?

If so, then returning an error from "turning on" the IC
return __kx022a_turn_on_off(data, true);
is problematic as fifo_buffer is already freed. New call to the disable 
would then cause a double free. If this is the case, it should be fixed, 
perhaps by just "NULLing" the fifo_buffer after the free.

If IIO does not allow "re-disable", then failures before the free() will 
cause a memory leak. Furthermore, the hardware state may be left 
inconsistent. Considering that such errors should be quite unexpected, 
and probably a symptom of shomething being off - I am not really 
terribly worried about the leak or inconsistent state - and I don't 
really know what else the driver could be except re-try - which might be 
a never ending story if problem persists.

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~