On Tue, 4 Mar 2025 09:36:56 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> On Tue, Mar 04, 2025 at 02:05:18PM +0800, sunliming@xxxxxxxxx wrote:
> > From: sunliming <sunliming@xxxxxxxxxx>
> >
> > Fix below kernel warning:
> > smatch warnings:
> > drivers/iio/imu/adis.c:319 __adis_check_status() error: uninitialized symbol 'status_16'.
> >
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> > Signed-off-by: sunliming <sunliming@xxxxxxxxxx>
>
> Huh... Someone is using lei to get their email. This patch is fine and
> it's theoretically the correct thing to do.
>
> How the zero-day bot warnings work is the they are first sent to my gmail
> account and I look them over and either forward them or ignore them. Here
> is the code:
>
> drivers/iio/imu/adis.c
> 305 int __adis_check_status(struct adis *adis)
> 306 {
> 307 unsigned int status;
> 308 int diag_stat_bits;
> 309 u16 status_16;
> 310 int ret;
> 311 int i;
> 312
> 313 if (adis->data->diag_stat_size) {
> 314 ret = adis->ops->read(adis, adis->data->diag_stat_reg, &status,
> 315 adis->data->diag_stat_size);
> 316 } else {
> 317 ret = __adis_read_reg_16(adis, adis->data->diag_stat_reg,
> 318 &status_16);
> 319 status = status_16;
> 320 }
> 321 if (ret)
> 322 return ret;
> 323
>
> So if __adis_read_reg_16() fails, then the next line is an uninitialized
> read. But then the if (ret) check means that it's fine at run-time.
> It's a false positive. The other thing to consider it the UBSan will
> also detect the uninitialized read at runtime and splat. That's still a
> false positive but it's a headache. But when I was looking at this, I
> decided that __adis_read_reg_16() was unlikely to fail in real life so I
> decided to ignore this warning.
>
> Initializing the variable to zero doesn't change runtime for sane configs
> because everyone automatically zeroes stack variables these days. It
> just silences the Smatch warning. So I'm fine with this patch.
>
> (This email is for information only in case you were wondering why the
> bug report was formatted strangely etc).
>
> regards,
> dan carpenter
>
Thanks! That explanation has me agreeing that this patch seems to
make sense as fixing a warning that is reasonable if unlikely to
cause problems in practice. I've applied it to the togreg branch of iio.git
Jonathan
