Message ID | 20191208.012032.1258816267132319518.davem@redhat.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [GIT] Networking | expand |
On Sun, Dec 8, 2019 at 1:20 AM David Miller <davem@redhat.com> wrote: > > git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git Grr, This introduces a new warning for me: drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c: In function ‘mlx5e_tc_tun_create_header_ipv6’: drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c:332:20: warning: ‘n’ may be used uninitialized in this function [-Wmaybe-uninitialized] 332 | struct neighbour *n; | ^ which is very annoying. The cause is commit 6c8991f41546 ("net: ipv6_stub: use ip6_dst_lookup_flow instead of ip6_dst_lookup") which changed mlx5e_route_lookup_ipv6() to use ipv6_dst_lookup_flow() which returns an error pointer, so it then does if (IS_ERR(dst)) return PTR_ERR(dst); instead of if (ret < 0) return ret; in the old code. And that then means that the caller, which does err = mlx5e_route_lookup_ipv6(priv, mirred_dev, &out_dev, &route_dev, &fl6, &n, &ttl); if (err) return err; and now gcc no longer sees that 'n' is always initialized when 'err' is zero. Because gcc apparently thinks that the if (IS_ERR(dst)) return PTR_ERR(dst); thing can result in a zero return value (I guess the cast from a 64-bit pointer to an 'int' is where it thinks "ok, that cast might lose high bits and become zero even when the original was tested to have a range where that will not happen). Anyway - the code is not buggy, but the new warning is simply not acceptable. We keep the build warning free even if it's gcc not being clever enough to see that "if it's uninitialized, we never get to the location where it's used". I don't know what the networking pattern for this is, but I did this in the merge -- struct neighbour *n; ++ struct neighbour *n = NULL; and I'm not happy about having gotten a pull request that has this kind of shit in it, especially since it was _known_ shit. It could have been that people had compilers that didn't see this problem. But no. I see that Stephen Rothwell reported it four days ago, and David seems to have even answered it. And yet that warning was still there in the pull request. WTF? David, this is not acceptable. You don't introduce new warnings in the tree. It doesn't matter one whit whether the warning is a false positive. A false positive warning will then be the cause of ignoring subsequent real warnings, which makes false positive warnings completely unacceptable. Fix your mindset, and stop sending me garbage. Linus
The pull request you sent on Sun, 08 Dec 2019 01:20:32 -0800 (PST):
> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git refs/heads/master
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/95e6ba5133163f8241c9ea2439369cec0452fec6
Thank you!
From: Linus Torvalds <torvalds@linux-foundation.org> Date: Sun, 8 Dec 2019 13:35:08 -0800 > Fix your mindset, and stop sending me garbage. Sorry. I was still brainstorming how to fix this properly.
Hi David, Komachi-san, On Sun, Dec 8, 2019 at 10:25 AM David Miller <davem@redhat.com> wrote: > Yoshiki Komachi (1): > cls_flower: Fix the behavior using port ranges with hw-offload I have bisected the below boot warning on m68k/ARAnyM to commit 8ffb055beae58574 ("cls_flower: Fix the behavior using port ranges with hw-offload"). Reverting the commit on top of v5.5-rc1 fixes the issue. WARNING: CPU: 0 PID: 7 at lib/refcount.c:28 refcount_warn_saturate+0x54/0x100 refcount_t: underflow; use-after-free. Modules linked in: CPU: 0 PID: 7 Comm: ksoftirqd/0 Not tainted 5.4.0-atari-10298-g8ffb055beae58574 #223 Stack from 00c31e88: 00c31e88 0038237c 00023d0a 00395a16 0000001c 00000009 00a67c80 00023d4e 00395a16 0000001c 001a7c30 00000009 00000000 00c31ed0 00000001 00000000 04208040 0000000a 00395a51 00c31ef0 00c30000 001a7c30 00395a16 0000001c 00000009 00395a51 0026b1bc 0032253c 00000003 003224f8 0026c2be 00000001 0032253c 00000000 00a67c80 00241d8c 00a67c80 00000000 00000200 000ab192 0004859e 00a67c80 0000000a 003facd8 003f5b90 002f2b46 003facd8 002f2dfe Call Trace: [<00023d0a>] __warn+0xb2/0xb4 [<00023d4e>] warn_slowpath_fmt+0x42/0x64 [<001a7c30>] refcount_warn_saturate+0x54/0x100 [<001a7c30>] refcount_warn_saturate+0x54/0x100 [<0026b1bc>] refcount_sub_and_test.constprop.73+0x38/0x3e [<0026c2be>] ipv4_dst_destroy+0x24/0x42 [<00241d8c>] dst_destroy+0x40/0xae [<000ab192>] kfree+0x0/0x3e [<0004859e>] rcu_process_callbacks+0x9a/0x9c [<002f2b46>] __do_softirq+0x146/0x182 [<002f2dfe>] schedule+0x0/0xb4 [<00035e76>] kthread_parkme+0x0/0x10 [<000359be>] __init_completion+0x0/0x20 [<00038308>] smpboot_thread_fn+0x0/0x100 [<00035fda>] kthread_should_stop+0x0/0x12 [<00035fce>] kthread_should_park+0x0/0xc [<00025b9c>] run_ksoftirqd+0x12/0x20 [<00038402>] smpboot_thread_fn+0xfa/0x100 [<00024888>] do_exit+0x0/0x6d4 [<0003f590>] complete+0x0/0x34 [<00036594>] kthread+0xb2/0xbc [<000359be>] __init_completion+0x0/0x20 [<000364e2>] kthread+0x0/0xbc [<00002a1c>] ret_from_kernel_thread+0xc/0x14 ---[ end trace 126b6dd25f47053b ]--- Gr{oetje,eeting}s, Geert