Re: [PATCH v2 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:
- [High] Missing `dev_set_drvdata()` in `max40080_probe()` causes a NULL/garbage pointer dereference in `max40080_resume()`.
- [Medium] Missing bounds check on hardware register read causes out-of-bounds array access in `max40080_get_oversampling_ratio()`.
--

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

This commit adds a direct-mode IIO driver for the MAX40080 bidirectional
current-sense amplifier. It exposes current and voltage channels, handles
selectable input ranges, and features PEC-protected register access.

Link: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX40080.pdf

> diff --git a/drivers/iio/adc/max40080.c b/drivers/iio/adc/max40080.c
> new file mode 100644
> index 0000000000000..87d2665151eda
> --- /dev/null
> +++ b/drivers/iio/adc/max40080.c

[ ... ]

> +static int max40080_get_oversampling_ratio(struct max40080_state *st, int *val)
> +{
> +	int tmp;
> +	u8 filter;
> +
> +	tmp = i2c_smbus_read_word_data(st->client, MAX40080_REG_CFG);
> +	if (tmp < 0)
> +		return tmp;
> +
> +	filter = FIELD_GET(MAX40080_CFG_FILTER_MSK, tmp);
> +	*val = max40080_oversampling_avail[filter];

[Severity: Medium]
Can filter exceed the bounds of the max40080_oversampling_avail array?

MAX40080_CFG_FILTER_MSK is GENMASK(14, 12), which allows values from 0 to 7.
However, the max40080_oversampling_avail array only has 6 elements.

If the device hardware enters an unexpected state or returns bits 6 or 7,
will this read past the end of the array and potentially leak kernel memory
to userspace?

> +
> +	return 0;
> +}

[ ... ]

> +static int max40080_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct max40080_state *st = iio_priv(indio_dev);
> +
> +	/*
> +	 * A suspend may have cut power to the device, resetting it to its
> +	 * power-on defaults. Reprogram it from the cached configuration.
> +	 */
> +	guard(mutex)(&st->lock);

[Severity: High]
Does max40080_probe() need to call dev_set_drvdata() before returning?

If the system suspends and resumes, max40080_resume() calls
dev_get_drvdata(dev) expecting an iio_dev pointer. But since max40080_probe()
doesn't set it, will this return the I2C core default (the i2c_client pointer)
instead?

If so, casting it to struct iio_dev * and calling iio_priv() will return a
garbage pointer for st, causing a crash when the lock is accessed here.

> +
> +	return max40080_init(st);
> +}

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