Re: [PATCH v5 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Interrupt storm on level-triggered lines and probe failure on nested IRQs due to hardirq request.
- [High] Time-of-Check to Time-of-Use (TOCTOU) heap buffer overflow when parsing device tree channels.
- [Medium] Hardware state desynchronization causing silent data corruption after system suspend.
--

--- Patch [2]: [PATCH v5 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04 ---
commit 60ab1f79461d9cde81c399c16a9e8fc0068a1dee
Author: Kyle Hsieh <kylehsieh1995@gmail.com>

iio: adc: ti-ads112c04: Add support for TI ADS112C04

Add IIO driver support for the Texas Instruments ADS112C04 (16-bit)
delta-sigma ADCs.

> diff --git a/drivers/iio/adc/ti-ads112c04.c b/drivers/iio/adc/ti-ads112c04.c
> index 0000000000000..c2995e94c4f8d
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads112c04.c
> @@ -166,15 +166,15 @@ static int ads112c04_get_adc_result(struct iio_dev *indio_dev,
> +	unsigned int idx = chan - indio_dev->channels;
> +	u8 new_config0, new_config1;
> +	int ret;
> +
> +	new_config0 = st->config0;
> +	FIELD_MODIFY(ADS112C04_CONF0_MUX, &new_config0, chan->address);
> +
> +	if (st->config0 != new_config0) {
> +		ret = ads112c04_write_reg(st->client, ADS112C04_REG_CONFIG0,
> +					  new_config0);
> +		if (ret < 0)
> +			return ret;
> +
> +		st->config0 = new_config0;
> +	}

[Severity: Medium]
Could this caching logic lead to silent data corruption after a system
suspend?

The driver skips writing the configuration if it matches the cached
value. Since there are no PM suspend/resume hooks, if the system
enters suspend and power to the ADC is cut, the hardware resets to its
Power-On Reset (POR) state.

Upon resume, if a conversion is requested that matches the cached
configuration, the driver will skip the I2C write. The hardware will
remain in its POR state and return incorrect measurements.

[ ... ]

> @@ -203,9 +203,9 @@ static irqreturn_t ads112c04_irq_handler(int irq, void *private)
> +static irqreturn_t ads112c04_irq_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct ads112c04_state *st = iio_priv(indio_dev);
> +
> +	complete(&st->completion);
> +
> +	return IRQ_HANDLED;
> +}

[ ... ]

> @@ -223,31 +223,31 @@ static int ads112c04_parse_channels(struct iio_dev *indio_dev,
> +static int ads112c04_parse_channels(struct iio_dev *indio_dev,
> +				    bool *need_avdd_ref, bool *need_ext_ref)
> +{
> +	struct device *dev = indio_dev->dev.parent;
> +	struct ads112c04_state *st = iio_priv(indio_dev);
> +	struct iio_chan_spec *channels;
> +	u32 num_channels, pair[2], channel;
> +	unsigned int i;
> +	int ret;
> +
> +	num_channels = device_get_named_child_node_count(dev, "channel");
> +	if (!num_channels)
> +		return dev_err_probe(dev, -EINVAL, "no channel subnodes found\n");
> +
> +	if (num_channels > ADS112C04_MAX_CHANNELS)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "num of channel nodes exceeds %d\n",
> +				     ADS112C04_MAX_CHANNELS);
> +
> +	channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;
> +
> +	st->vref_source = devm_kcalloc(dev, num_channels,
> +				       sizeof(*st->vref_source), GFP_KERNEL);
> +	if (!st->vref_source)
> +		return -ENOMEM;
> +
> +	i = 0;
> +	device_for_each_named_child_node_scoped(dev, child, "channel") {
> +		struct iio_chan_spec *spec = &channels[i];

[Severity: High]
Can this cause an out-of-bounds heap write?

The driver counts the number of child nodes to allocate the arrays, and
then iterates over the child nodes to populate them in two separate
traversals.

If a device tree overlay is applied concurrently between the count and
the iteration, the number of child nodes can increase, causing the loop
index 'i' to exceed num_channels.

[ ... ]

> @@ -493,5 +493,5 @@ static int ads112c04_probe(struct i2c_client *client)
> +	if (client->irq > 0) {
> +		ret = devm_request_irq(dev, client->irq, ads112c04_irq_handler, 0,
> +				       indio_dev->name, indio_dev);

[Severity: High]
Will this cause an interrupt storm if the DRDY interrupt is configured
as level-triggered?

Since this is requested as a hard IRQ via devm_request_irq(), the
handler completes the completion and returns IRQ_HANDLED without
clearing the hardware interrupt. The line will remain asserted.

Also, if the ADC is connected to a slow bus (like an I2C GPIO
expander) which provides a nested threaded IRQ, will this fail with
-EINVAL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813-ti-ads112c04-driver-v5-0-79dff9e249cd@gmail.com?part=2