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] [thread-next>] [day] [month] [year] [list]

Message-ID: <c5b70fd8-2d03-4179-a8b8-5ee827fff978@stanley.mountain>
Date: Tue, 4 Mar 2025 09:36:56 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: sunliming@...ux.dev
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, nuno.sa@...log.com,
	jic23@...nel.org, 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, 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