Re: [PATCH v4 3/3] iio: adc: ti-ads112c14: add continuous mode support

[David Lechner <dlechner@baylibre.com>]

On 8/30/26 5:24 PM, Jonathan Cameron wrote:
> On Sun, 30 Aug 2026 22:00:34 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 

...

>>
>>> +static int ads112c14_buffer_postenable(struct iio_dev *indio_dev)
>>> +{
>>> +	struct ads112c14_data *data = iio_priv(indio_dev);
>>> +	const struct iio_chan_spec *chan;
>>> +	int ret;
>>> +
>>> +	if (!ads112c14_using_drdy_trigger(indio_dev))
>>> +		return 0;
>>> +
>>> +	chan = ads112c14_first_active_channel(indio_dev);
>>> +	if (!chan)
>>> +		return -EINVAL;
>>> +
>>> +	guard(mutex)(&data->lock);
>>> +
>>> +	ret = ads112c14_prepare_channel(data, chan);  
>>
>  | sashiko.dev <sashiko@sashiko.dev>:
>  |
>  | [Severity: High]
>  | Can a user bypass the single-channel limitation and push corrupted data
>  | to the ring buffer here?
>  |
>  | Since ads112c14_validate_scan_mask() allows multiple channels if a non-DRDY
>  | trigger is currently selected, a user could enable multiple channels via
>  | scan_elements, then switch current_trigger to the DRDY trigger before
>  | enabling the IIO buffer.
> 
>>
>> Hmm. Indeed an interesting corner...  From a quick look it is correct.
>> I'm not sure if this is in practice an existing bug - do other
>> drivers change acceptable channels based on another change such as
>> which trigger is in use?   Either way we need to close this.
>>
>> We can't just go clearing the set bits on setting the trigger as
>> that might cause a regression.  So I think all we can do is add
>> a well commented additional check early in the buffer enable path.
>>
>> Given the behaviour that is causing problems is present in this
>> driver we could either add the protection in fix and rely on that
>> going upstream first, or add it as first patch in this series and
>> let it work its way upstream with this patch.
>>
>> Nice catch to sashiko!
>>
> 
>  |
>  | Because this driver does not provide an available_scan_masks array, the
>  | mask isn't re-validated during buffer enablement. Since
>  | ads112c14_buffer_postenable() only configures the first active channel,
>  | will the trigger handler push uninitialized heap memory for the remaining
>  | channels when the full array is sent to userspace?
>  |
>  | via: https://sashiko.dev/#/message/20260827-iio-adc-ti-ads112c14-continuous-mode-v4-3-1e51a6e20a69@baylibre.com
> 
Ah, I had it mixed up in my mind with update_scan_mode(), which would be
called at the appropriate time.