[PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp

Salah Triki posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
drivers/iio/adc/mt6359-auxadc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp
Posted by Salah Triki 1 month, 2 weeks ago
In mt6358_read_imp(), the return value of regmap_read() is currently
ignored. This is problematic because if the bus read fails the variable
val_v remains uninitialized.

The function subsequently assigns this uninitialized stack value to
*vbat, leading to incorrect measurement results being reported to
the IIO subsystem without any error indication.

Update the function to check the return value of regmap_read(). Ensure
that mt6358_stop_imp_conv() is still called to clean up the hardware
state before returning the error code.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 drivers/iio/adc/mt6359-auxadc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mt6359-auxadc.c b/drivers/iio/adc/mt6359-auxadc.c
index 6b9ed9b1fde2..f927bff4a26a 100644
--- a/drivers/iio/adc/mt6359-auxadc.c
+++ b/drivers/iio/adc/mt6359-auxadc.c
@@ -497,10 +497,13 @@ static int mt6358_read_imp(struct mt6359_auxadc *adc_dev,
 		return ret;
 
 	/* Read the params before stopping */
-	regmap_read(regmap, reg_adc0 + (cinfo->imp_adc_num << 1), &val_v);
+	ret = regmap_read(regmap, reg_adc0 + (cinfo->imp_adc_num << 1), &val_v);
 
 	mt6358_stop_imp_conv(adc_dev);
 
+	if (ret)
+		return ret;
+
 	if (vbat)
 		*vbat = val_v;
 	if (ibat)
-- 
2.43.0
Re: [PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Mon, Apr 27, 2026 at 09:54:57AM +0100, Salah Triki wrote:
> In mt6358_read_imp(), the return value of regmap_read() is currently
> ignored. This is problematic because if the bus read fails the variable
> val_v remains uninitialized.
> 
> The function subsequently assigns this uninitialized stack value to
> *vbat, leading to incorrect measurement results being reported to
> the IIO subsystem without any error indication.
> 
> Update the function to check the return value of regmap_read(). Ensure
> that mt6358_stop_imp_conv() is still called to clean up the hardware
> state before returning the error code.

Sounds like this deserves a Fixes tag, but the problem is that the whole driver
is written like this. Why does having this fixed make it special?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp
Posted by Salah Triki 1 month, 2 weeks ago
On Mon, Apr 27, 2026 at 12:14:40PM +0300, Andy Shevchenko wrote:
> On Mon, Apr 27, 2026 at 09:54:57AM +0100, Salah Triki wrote:
> > In mt6358_read_imp(), the return value of regmap_read() is currently
> > ignored. This is problematic because if the bus read fails the variable
> > val_v remains uninitialized.
> > 
> > The function subsequently assigns this uninitialized stack value to
> > *vbat, leading to incorrect measurement results being reported to
> > the IIO subsystem without any error indication.
> > 
> > Update the function to check the return value of regmap_read(). Ensure
> > that mt6358_stop_imp_conv() is still called to clean up the hardware
> > state before returning the error code.
> 
> Sounds like this deserves a Fixes tag, but the problem is that the whole driver
> is written like this. Why does having this fixed make it special?
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

You are right, Andy. I noticed that several other functions in this driver
share the same issue. I will send a V2 that addresses all unchecked
regmap_read() calls in this file and I will include the Fixes tag. Thanks
for the feedback.

Best Regards
Salah Triki
Re: [PATCH] iio: adc: mt6359: fix unchecked return value in mt6358_read_imp
Posted by Jonathan Cameron 1 month, 2 weeks ago
On Mon, 27 Apr 2026 12:31:34 +0100
Salah Triki <salah.triki@gmail.com> wrote:

> On Mon, Apr 27, 2026 at 12:14:40PM +0300, Andy Shevchenko wrote:
> > On Mon, Apr 27, 2026 at 09:54:57AM +0100, Salah Triki wrote:  
> > > In mt6358_read_imp(), the return value of regmap_read() is currently
> > > ignored. This is problematic because if the bus read fails the variable
> > > val_v remains uninitialized.
> > > 
> > > The function subsequently assigns this uninitialized stack value to
> > > *vbat, leading to incorrect measurement results being reported to
> > > the IIO subsystem without any error indication.
> > > 
> > > Update the function to check the return value of regmap_read(). Ensure
> > > that mt6358_stop_imp_conv() is still called to clean up the hardware
> > > state before returning the error code.  
> > 
> > Sounds like this deserves a Fixes tag, but the problem is that the whole driver
> > is written like this. Why does having this fixed make it special?
> > 
> > -- 
> > With Best Regards,
> > Andy Shevchenko
> > 
> >   
> 
> You are right, Andy. I noticed that several other functions in this driver
> share the same issue. I will send a V2 that addresses all unchecked
> regmap_read() calls in this file and I will include the Fixes tag. Thanks
> for the feedback.
What's the bus in this case?

Whilst maybe we should be checking them, it is not uncommon to forgo those
checks in internal busses.  In general they should be more reliable than
something going over PCB tracks like spi or i2c.

I'd be tempted to fix the uninitialized variable by setting it at declaration
rather than worry too much about checking all the regmap return values.

> 
> Best Regards
> Salah Triki