Re: [PATCH v2 3/4] iio: accel: kionix-kx022a: Prevent memory leak and fix state

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 28/08/2026 10:56, Andy Shevchenko wrote:
> 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.

As far as I can see, the driver is consistently using the 'data->regmap' 
everywhere. Using 'map' just in one function would raise question if it 
is different regmap from every other place.

I am 100% Ok with anyone changing the data->regmap to 'map' throughout 
the whole driver in order to cut the amount of lines - but it is not 
something to be done in a bugfix commit.

Yours,
	-- Matti

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

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