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

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 14 Aug 2026 10:39:48 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> 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 <mazziesaccount@gmail.com>
> >>
> >> 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 <mehdi.djait@linux.intel.com>
> >   
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> >> 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)?

It carries on when it gets a disable on the buffer tear down path.
It isn't obvious what the right choice is and we can't really just
back off and let things try again as some of the reasons this occurs
are things like driver unbind which doesn't fail.

So best effort for error handling only on the tear down + ideally
some prints so we know what has gone wrong if it does happen.

Writing a driver that survives random bus transmission failures in
every uncommon path is really hard and the complexity is often
not worthwhile.

Jonathan


> 
> 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
>