In case the conversion has failed or returned zero, processing *val
can lead to a division by zero.
Need to check for errors, or converted value is zero, before processing
the data.
In case converted value is zero, e.g. the Vrefint channel, this should
be considered as invalid in all case.
Fixes: 0e346b2cfa85 ("iio: adc: stm32-adc: add vrefint calibration support")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/all/20260911161555.244F31F000FF@smtp.kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
drivers/iio/adc/stm32-adc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
From: Andy Shevchenko <hidden> Date: 2026-09-16 07:35:54
On Tue, Sep 15, 2026 at 06:10:40PM +0200, Fabrice Gasnier wrote:
In case the conversion has failed or returned zero, processing *val
can lead to a division by zero.
Need to check for errors, or converted value is zero, before processing
the data.
In case converted value is zero, e.g. the Vrefint channel, this should
be considered as invalid in all case.
Something went very wrong with the indentation of the above.
...
- if (mask == IIO_CHAN_INFO_PROCESSED)
- *val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
+ if (mask == IIO_CHAN_INFO_PROCESSED) {
+ if (ret >= 0 && *val)
+ *val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
+ else
+ ret = ret < 0 ? ret : -EINVAL;
+ }
Reuse of the *val makes code harder to follow. Add a temporary variable for
this and do something like this (also note other simplifications)
tmp_choose_good_name = *val;
...
if (mask == IIO_CHAN_INFO_PROCESSED) {
if (ret < 0)
return ret;
if (tmp == 0)
return -EINVAL;
*val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / tmp;
}
On Tue, Sep 15, 2026 at 06:10:40PM +0200, Fabrice Gasnier wrote:
quoted
In case the conversion has failed or returned zero, processing *val
can lead to a division by zero.
Need to check for errors, or converted value is zero, before processing
the data.
In case converted value is zero, e.g. the Vrefint channel, this should
be considered as invalid in all case.
Something went very wrong with the indentation of the above.
Hi Andy,
Euh, sorry but I don't understand what's wrong with indentation in the
commit message ? Could you clarify ?
...
quoted
- if (mask == IIO_CHAN_INFO_PROCESSED)
- *val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
+ if (mask == IIO_CHAN_INFO_PROCESSED) {
+ if (ret >= 0 && *val)
+ *val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
+ else
+ ret = ret < 0 ? ret : -EINVAL;
+ }
Reuse of the *val makes code harder to follow. Add a temporary variable for
this and do something like this (also note other simplifications)
tmp_choose_good_name = *val;
In case ret is an error; this lead to evaluate (e.g. read) *val from the
(maybe uninitialized) stack. Probably not an issue? But would prefer to
keep reading it only when ret >= 0, as done currently with the if
condition above ? (e.g. evaluate ret, before *val)
...
if (mask == IIO_CHAN_INFO_PROCESSED) {
if (ret < 0)
return ret;
if (tmp == 0)
return -EINVAL;
I've started with something similar before, but we can't return directly
here. Must call iio_device_release_direct() first. This would add more
lines.
So I chose to implement above ternary ret = ret < 0 ? ret : -EINVAL, to
fall-through.
Please advise on the preferred way,
Best Regards,
Fabrice
From: Andy Shevchenko <hidden> Date: 2026-09-16 11:00:19
On Wed, Sep 16, 2026 at 12:23:38PM +0200, Fabrice Gasnier wrote:
On 9/16/26 09:35, Andy Shevchenko wrote:
quoted
On Tue, Sep 15, 2026 at 06:10:40PM +0200, Fabrice Gasnier wrote:
quoted
In case the conversion has failed or returned zero, processing *val
can lead to a division by zero.
Need to check for errors, or converted value is zero, before processing
the data.
In case converted value is zero, e.g. the Vrefint channel, this should
be considered as invalid in all case.
Something went very wrong with the indentation of the above.
Hi Andy,
Euh, sorry but I don't understand what's wrong with indentation in the
commit message ? Could you clarify ?
Each sentence seems to be a separate paragraph. Paragraphs are delimited
by a blank lines. But are they (sentences) really so independent?
Compare to:
In case the conversion has failed or returned zero, processing *val
can lead to a division by zero. Need to check for errors, or converted
value is zero, before processing the data. In case converted value
is zero, e.g. the Vrefint channel, this should be considered as invalid
in all cases.
(also I fixed the typo, should be plural for 'cases' at the end).
...
quoted
quoted
- if (mask == IIO_CHAN_INFO_PROCESSED)
- *val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
+ if (mask == IIO_CHAN_INFO_PROCESSED) {
+ if (ret >= 0 && *val)
+ *val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
+ else
+ ret = ret < 0 ? ret : -EINVAL;
+ }
Reuse of the *val makes code harder to follow. Add a temporary variable for
this and do something like this (also note other simplifications)
tmp_choose_good_name = *val;
In case ret is an error; this lead to evaluate (e.g. read) *val from the
(maybe uninitialized) stack. Probably not an issue? But would prefer to
keep reading it only when ret >= 0, as done currently with the if
condition above ? (e.g. evaluate ret, before *val)
quoted
...
if (mask == IIO_CHAN_INFO_PROCESSED) {
if (ret < 0)
return ret;
Then simply move the assignment here
tmp_choose_good_name = *val;
quoted
if (tmp == 0)
return -EINVAL;
I've started with something similar before, but we can't return directly
here. Must call iio_device_release_direct() first. This would add more
lines.
So I chose to implement above ternary ret = ret < 0 ? ret : -EINVAL, to
fall-through.
Please advise on the preferred way,