Re: [PATCH v4 2/2] iio: adc: add MAX40080 current-sense amplifier driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Uninitialized stack variable `tmp` is read and assigned to `*iv` on the error path in `max40080_read_iv`.
- [Medium] Unprotected read of `st->range` in `max40080_read_raw` races with concurrent modifications.
--

--- Patch [2]: [PATCH v4 2/2] iio: adc: add MAX40080 current-sense amplifier driver ---
commit b542cce161e1d593b12f173fe74a0a6612fdcb77
Author: Stefan Popa <stefan.popa@analog.com>

iio: adc: add MAX40080 current-sense amplifier driver

This commit adds a new driver for the Analog Devices MAX40080 bidirectional
current-sense amplifier. It exposes current and voltage channels via IIO and
allows configuring oversampling and input ranges through I2C.

diff --git a/drivers/iio/adc/max40080.c b/drivers/iio/adc/max40080.c
new file mode 100644
--- /dev/null
+++ b/drivers/iio/adc/max40080.c
[ ... ]
> +static int max40080_read_iv(struct max40080_state *st, u32 *iv)
> +{
> +	struct i2c_client *client = st->client;
> +	int ret, io_ret;
> +	u32 tmp;
[ ... ]
> +	ret = read_poll_timeout(max40080_read_iv_once, io_ret,
> +				io_ret || (tmp & MAX40080_IV_VALID_MSK),
> +				1 * USEC_PER_MSEC, 50 * USEC_PER_MSEC,
> +				false, st, &tmp);
> +	/*
> +	 * Update *iv even on timeout — the caller checks ret first, but the
> +	 * last-read value (with valid bit still clear) can be useful for debug.
> +	 */
> +	*iv = tmp;

[Severity: Medium]
If max40080_read_iv_once() fails on the first attempt and returns a negative
error code (like -EIO), read_poll_timeout() will short-circuit without
evaluating the right side of the condition.

Will this leave tmp uninitialized, resulting in uninitialized stack memory
being copied to *iv before the error is returned?

[ ... ]
> +static int max40080_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct max40080_state *st = iio_priv(indio_dev);
> +	unsigned int range;
> +	int ret;
> +
> +	switch (mask) {
[ ... ]
> +	case IIO_CHAN_INFO_SCALE:
> +		/*
> +		 * The selectable current-sense range is exposed through
> +		 * scale: each RANGE setting has its own precomputed
> +		 * mA-per-code value. Userspace picks the range by writing
> +		 * the matching scale.
> +		 */
> +		switch (chan->type) {
> +		case IIO_CURRENT:
> +			range = st->range;

[Severity: Medium]
Since max40080_set_range() updates st->range while holding st->lock,
could reading it here without taking the lock cause a data race if a user
concurrently reads and writes the scale sysfs attribute?

> +			*val = st->current_scale[range][0];
> +			*val2 = st->current_scale[range][1];
> +			return IIO_VAL_INT_PLUS_NANO;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717123852.1140893-1-stefan.popa@analog.com?part=2