Re: [linux-next:master 3206/7719] drivers/iio/adc/ad4030.c:378 ad4030_get_chan_scale() error: 'scan_type' dereferencing possible ERR_PTR()

Re: [linux-next:master 3206/7719] drivers/iio/adc/ad4030.c:378 ad4030_get_chan_scale() error: 'scan_type' dereferencing possible ERR_PTR()

[Jonathan Cameron]

On Tue, 4 Mar 2025 09:22:43 +0300
Dan Carpenter <dan.carpenter@linaro.org> wrote:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   cd3215bbcb9d4321def93fea6cfad4d5b42b9d1d
> commit: 949abd1ca5a4342d9fb8c774035222e65dd1cfe4 [3206/7719] iio: adc: ad4030: add averaging support
> config: openrisc-randconfig-r071-20250304 (https://download.01.org/0day-ci/archive/20250304/202503040954.n6MhjSsV-lkp@intel.com/config)
> compiler: or1k-linux-gcc (GCC) 14.2.0
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202503040954.n6MhjSsV-lkp@intel.com/
> 
> smatch warnings:
> drivers/iio/adc/ad4030.c:378 ad4030_get_chan_scale() error: 'scan_type' dereferencing possible ERR_PTR()
> 
> vim +/scan_type +378 drivers/iio/adc/ad4030.c
> 
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  366  static int ad4030_get_chan_scale(struct iio_dev *indio_dev,
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  367  				 struct iio_chan_spec const *chan,
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  368  				 int *val,
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  369  				 int *val2)
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  370  {
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  371  	struct ad4030_state *st = iio_priv(indio_dev);
> 949abd1ca5a4342 Esteban Blanc 2025-02-14  372  	const struct iio_scan_type *scan_type;
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  373  
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  374  	if (chan->differential) {
> 949abd1ca5a4342 Esteban Blanc 2025-02-14  375  		scan_type = iio_get_current_scan_type(indio_dev,
> 949abd1ca5a4342 Esteban Blanc 2025-02-14  376  						      st->chip->channels);
> 
> 
> Do we need an if (IS_ERR(scan_type)) return;?
No but it is a bit of chase to establish why not so I think it would be sensible from
the point of view of resilience and readability to add that check.

That function can fail with an ERR_PTR() if either the callback it uses returns an error
(the one in this driver never does) or the value is out of range (I assume with only 2 values
we manage to avoid that too though I didn't actually check).

It's not a high performance path to read this so the check would be a good addition but
I'm not going to rush in the change.  Esteban, would you mind spinning a patch to add
the check if you agree with the reasoning?

+CC linux-iio for wider visibility.

Thanks,

Jonathan

> 
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  377  		*val = (st->vref_uv * 2) / MILLI;
> 949abd1ca5a4342 Esteban Blanc 2025-02-14 @378  		*val2 = scan_type->realbits;
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  379  		return IIO_VAL_FRACTIONAL_LOG2;
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  380  	}
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  381  
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  382  	*val = st->vref_uv / MILLI;
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  383  	*val2 = chan->scan_type.realbits;
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  384  	return IIO_VAL_FRACTIONAL_LOG2;
> 0cb8b324852f3e3 Esteban Blanc 2025-02-14  385  }
>