linux-kernel - Re: [PATCH] iio: imu: adis: fix uninitialized symbol warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250304141518.7389ef88@jic23-huawei>
Date: Tue, 4 Mar 2025 14:15:18 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: sunliming@...ux.dev, lars@...afoo.de, Michael.Hennerich@...log.com,
 nuno.sa@...log.com, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, sunliming@...inos.cn, kernel test robot
 <lkp@...el.com>, Dan Carpenter <error27@...il.com>
Subject: Re: [PATCH] iio: imu: adis: fix uninitialized symbol warning

On Tue, 4 Mar 2025 09:36:56 +0300
Dan Carpenter <dan.carpenter@...aro.org> wrote:

> On Tue, Mar 04, 2025 at 02:05:18PM +0800, sunliming@...ux.dev wrote:
> > From: sunliming <sunliming@...inos.cn>
> > 
> > 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@...el.com>
> > Reported-by: Dan Carpenter <error27@...il.com>
> > Signed-off-by: sunliming <sunliming@...inos.cn>  
> 
> 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