Re: [kbuild] [kees:kspp/uninit/macro 9/10] drivers/virtio/virtio_ring.c:1197 virtqueue_add_packed() error: uninitialized symbol 'prev'.
7:20 a.m.
On Fri, Jun 05, 2020 at 01:38:20PM +0300, kernel test robot wrote:
Hey Kees,
Some of these introduce static checker false positives. Which is fine.
The question is, will they eventually introduce GCC warnings or will
the automatic initialization code always zero them out?
Well, I think the issue is mainly with the compilers/checkers. Using
uninitialized_var() means we can't ever get them correctly analyzed, so
we have to remove them. The best way to silence them is to explicitly
initialize them, and that's really up to the individual code use.
The auto-init code does force-initialize them, though the value is
different between the GCC plugin and Clang (currently). I expect to
switch the kernel to always zero with Clang, but there is some debate
about feature naming, etc[1].
I have been seeing a ton of uninitialized variable bugs recently.
Quite
often it's fine because zero is correct, but sometimes it leads to a
NULL pointer dereference.
Right -- but better it be a deterministic failure than something under
the influence of stale stack contents.
tree:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git kspp/uninit/macro
head: 806e8c03bcab6ac1f1ea142980de222fa021a75c
commit: fc9fcebe481c0e849a8141281411613eae76bd29 [9/10] treewide: Remove
uninitialized_var() usage
config: x86_64-randconfig-m001-20200604 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp(a)intel.com>
Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com>
smatch warnings:
drivers/virtio/virtio_ring.c:1197 virtqueue_add_packed() error: uninitialized symbol
'prev'.
drivers/virtio/virtio_ring.c:1205 virtqueue_add_packed() error: uninitialized symbol
'head_flags'.
drivers/virtio/virtio_ring.c:508 virtqueue_add_split() error: uninitialized symbol
'prev'.
Regardless, I'll see if I can get this explicitly initialized to avoid
the new smatch warnings.
Thanks for the report! :)
-Kees
[1] https://lists.llvm.org/pipermail/cfe-dev/2020-April/065221.html
--
Kees Cook
9:38 p.m.
New subject: [kbuild] [kees:kspp/uninit/macro 9/10] drivers/virtio/virtio_ring.c:1197 virtqueue_add_packed() error: uninitialized symbol 'prev'.
On Fri, Jun 05, 2020 at 02:20:12PM -0700, Kees Cook wrote:
On Fri, Jun 05, 2020 at 01:38:20PM +0300, kernel test robot wrote:
> smatch warnings:
> drivers/virtio/virtio_ring.c:1197 virtqueue_add_packed() error: uninitialized symbol
'prev'.
> drivers/virtio/virtio_ring.c:1205 virtqueue_add_packed() error: uninitialized symbol
'head_flags'.
> drivers/virtio/virtio_ring.c:508 virtqueue_add_split() error: uninitialized symbol
'prev'.
Regardless, I'll see if I can get this explicitly initialized to avoid
the new smatch warnings.
I would prefer to leave it as-is rather than silencing it just for
Smatch. Eventually Smatch will be able to parse these things correctly.
My concern is that eventually GCC will re-enable their uninitialized
variable warning and we will be flooded with warnings. Maybe when that
happens we will disable the GCC check and only rely on Smatch, though so
maybe that's a good thing.
regards,
dan carpenter
834
days inactive
837
days old
1 comments
2 participants
participants (2)
-
Dan Carpenter -
Kees Cook