Re: [PATCH v2 3/4] iio: accel: kionix-kx022a: Prevent memory leak and fix state
Andy Shevchenko <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel |
|---|---|
| Organization | Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo |
| Message-ID | <[email protected]> |
On Fri, Aug 28, 2026 at 10:40:51AM +0300, Matti Vaittinen wrote: > 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. ... > static int kx022a_fifo_enable(struct kx022a_data *data) With struct regmap *map = data->regmap; > guard(mutex)(&data->mutex); > ret = __kx022a_turn_on_off(data, false); > if (ret) > - return ret; > + goto err_free_out; > > /* Update watermark to HW */ > ret = kx022a_fifo_set_wmi(data); > if (ret) > - return ret; > + goto err_wmi_out; > > /* Enable buffer */ > ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2, > KX022A_MASK_BUF_EN); > if (ret) > - return ret; > + goto err_wmi_out; > > data->state |= KX022A_STATE_FIFO; > ret = regmap_set_bits(data->regmap, data->ien_reg, > KX022A_MASK_WMI); > if (ret) > - return ret; > + goto err_buf_en_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); in particular this will be regmap_clear_bits(map, data->ien_reg, KX022A_MASK_WMI); > +err_buf_en_out: > + regmap_clear_bits(data->regmap, data->chip_info->buf_cntl2, > + KX022A_MASK_BUF_EN); regmap_clear_bits(map, data->chip_info->buf_cntl2, KX022A_MASK_BUF_EN); Which saves a single line, and the followup may shorten the existing code even more. > + data->state &= ~KX022A_STATE_FIFO; > +err_wmi_out: > + __kx022a_turn_on_off(data, true); > +err_free_out: > + kfree(data->fifo_buffer); > + > + return ret; > } -- With Best Regards, Andy Shevchenko