Re: [PATCH 2/2] iio: temperature: tmp117: add support for HIGH and LOW alerts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] tmp117: use-after-free of IIO event interface on probe error path
- [Low] tmp117: build failure on 32-bit platforms due to 64-bit division
--

commit 986d5decf91950670647d2b2db83a43e9c93c02f
Author: Thomas Bonnefille <thomas.bonnefille@bootlin.com>

iio: temperature: tmp117: add support for HIGH and LOW alerts

This commit adds support for setting high and low temperature alert limits on
TMP11x chips. It configures an interrupt handler to generate IIO events when
these limits are crossed.

> diff --git a/drivers/iio/temperature/tmp117.c b/drivers/iio/temperature/tmp117.c
> index 74cb8d62bef3c..d2924e9a7f97b 100644
> --- a/drivers/iio/temperature/tmp117.c
> +++ b/drivers/iio/temperature/tmp117.c
[ ... ]
> @@ -52,6 +62,112 @@ struct tmp11x_info {
[ ... ]
> +static int tmp117_write_thresh(struct iio_dev *indio_dev,
> +			       const struct iio_chan_spec *chan,
> +			       enum iio_event_type type,
> +			       enum iio_event_direction dir,
> +			       enum iio_event_info info,
> +			       int val, int val2)
> +{
[ ... ]
> +	tmp = (val < 0) ? (s64)val * 1000000 - val2
> +			: (s64)val * 1000000 + val2;
> +	tmp = DIV_ROUND_CLOSEST(tmp * TMP117_LIMIT_LSB_DEN,
> +				(s64)TMP117_LIMIT_LSB_NUM * 1000000);

[Severity: Low]
Does this macro expansion cause a build failure on 32-bit platforms?

DIV_ROUND_CLOSEST natively invokes the / operator. Passing two 64-bit integers
here causes a 64-bit division, which generates an undefined reference to
__divdi3 on 32-bit builds.

> +	tmp = clamp_t(s64, tmp, S16_MIN, S16_MAX);
> +
> +	return i2c_smbus_write_word_swapped(data->client,
> +					    reg, tmp);
> +}
[ ... ]
> @@ -208,6 +343,16 @@ static int tmp117_probe(struct i2c_client *client)
[ ... ]
> +	if (client->irq) {
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +						NULL, tmp117_interrupt_handler,
> +						IRQF_ONESHOT, match_data->name,
> +						indio_dev);
> +		if (ret) {
> +			dev_err_probe(&client->dev, ret, "irq request error\n");
> +			return ret;
> +		}
> +	}
>  
>  	return devm_iio_device_register(&client->dev, indio_dev);
>  }

[Severity: High]
Can this sequence lead to a use-after-free of the IIO event interface?

If devm_iio_device_register() fails, its error path frees the IIO event
interface. However, because the interrupt was already enabled by
devm_request_threaded_irq(), it remains active until devres tears it down.

If an interrupt triggers during this window, tmp117_interrupt_handler()
executes:

tmp117_interrupt_handler()
    iio_push_event(indio_dev, ...)

Will iio_push_event() dereference the already freed event interface structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-add-tmp117-notif-v1-0-c6cef6fad64c@bootlin.com?part=2