Re: [PATCH 4.14 43/43] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
On Sat, Aug 03, 2019 at 08:45:03AM +0800, Xin Long wrote: > On Fri, Aug 2, 2019 at 7:03 PM Rantala, Tommi T. (Nokia - FI/Espoo) > wrote: > > > > On Fri, 2019-08-02 at 09:28 +0200, gre...@linuxfoundation.org wrote: > > > On Thu, Aug 01, 2019 at 10:17:30AM +, Rantala, Tommi T. (Nokia - > > > FI/Espoo) wrote: > > > > Hi, > > > > > > > > This tipc patch added in 4.14.132 is triggering a crash for me, > > > > revert > > > > fixes it. > > > > > > > > Anyone have ideas if some other commits missing in 4.14.x to make > > > > this > > > > work...? > > > > > > Do you also hav a problem with 4.19.y? How about 5.2.y? If not, can > > > you do 'git bisect' to find the patch that fixes the issue? > > > > > > thanks, > > > > > > greg k-h > > > > Hi, please pick this to 4.14.y and 4.19.y, tested that it fixes the > > crash in both: > > > > commit 5684abf7020dfc5f0b6ba1d68eda3663871fce52 > > Author: Xin Long > > Date: Mon Jun 17 21:34:13 2019 +0800 > > > > ip_tunnel: allow not to count pkts on tstats by setting skb's dev > > to NULL > Thanks Rantala, > > sorry for late, I was in a trip. > > The patch belongs to a patchset: > > https://www.spinics.net/lists/netdev/msg578674.html > > So this commit should also be included: > > commit 6f6a8622057c92408930c31698394fae1557b188 > Author: Xin Long > Date: Mon Jun 17 21:34:14 2019 +0800 > > ip6_tunnel: allow not to count pkts on tstats by passing dev as NULL This commit is also included in the following kernel releases: 4.9.186 4.14.134 4.19.59 5.1.18 5.2 so this should all be taken care of, right? If not, please let me know. thanks, greg k-h
Re: [PATCH 4.14 43/43] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
On Fri, Aug 2, 2019 at 7:03 PM Rantala, Tommi T. (Nokia - FI/Espoo) wrote: > > On Fri, 2019-08-02 at 09:28 +0200, gre...@linuxfoundation.org wrote: > > On Thu, Aug 01, 2019 at 10:17:30AM +, Rantala, Tommi T. (Nokia - > > FI/Espoo) wrote: > > > Hi, > > > > > > This tipc patch added in 4.14.132 is triggering a crash for me, > > > revert > > > fixes it. > > > > > > Anyone have ideas if some other commits missing in 4.14.x to make > > > this > > > work...? > > > > Do you also hav a problem with 4.19.y? How about 5.2.y? If not, can > > you do 'git bisect' to find the patch that fixes the issue? > > > > thanks, > > > > greg k-h > > Hi, please pick this to 4.14.y and 4.19.y, tested that it fixes the > crash in both: > > commit 5684abf7020dfc5f0b6ba1d68eda3663871fce52 > Author: Xin Long > Date: Mon Jun 17 21:34:13 2019 +0800 > > ip_tunnel: allow not to count pkts on tstats by setting skb's dev > to NULL Thanks Rantala, sorry for late, I was in a trip. The patch belongs to a patchset: https://www.spinics.net/lists/netdev/msg578674.html So this commit should also be included: commit 6f6a8622057c92408930c31698394fae1557b188 Author: Xin Long Date: Mon Jun 17 21:34:14 2019 +0800 ip6_tunnel: allow not to count pkts on tstats by passing dev as NULL Next time I think I should put "Fixes:" flag into each patch. > > > For 5.2.y nothing is needed, these commits were in v5.2-rc6 already. > > -Tommi >
Re: [PATCH 4.14 43/43] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
On Fri, Aug 02, 2019 at 11:03:15AM +, Rantala, Tommi T. (Nokia - FI/Espoo) wrote: > On Fri, 2019-08-02 at 09:28 +0200, gre...@linuxfoundation.org wrote: > > On Thu, Aug 01, 2019 at 10:17:30AM +, Rantala, Tommi T. (Nokia - > > FI/Espoo) wrote: > > > Hi, > > > > > > This tipc patch added in 4.14.132 is triggering a crash for me, > > > revert > > > fixes it. > > > > > > Anyone have ideas if some other commits missing in 4.14.x to make > > > this > > > work...? > > > > Do you also hav a problem with 4.19.y? How about 5.2.y? If not, can > > you do 'git bisect' to find the patch that fixes the issue? > > > > thanks, > > > > greg k-h > > Hi, please pick this to 4.14.y and 4.19.y, tested that it fixes the > crash in both: > > commit 5684abf7020dfc5f0b6ba1d68eda3663871fce52 > Author: Xin Long > Date: Mon Jun 17 21:34:13 2019 +0800 > > ip_tunnel: allow not to count pkts on tstats by setting skb's dev > to NULL > > > For 5.2.y nothing is needed, these commits were in v5.2-rc6 already. Thanks for the info, now snuck into the latest round of -rc releases. greg k-h
Re: [PATCH 4.14 43/43] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
On Fri, 2019-08-02 at 09:28 +0200, gre...@linuxfoundation.org wrote: > On Thu, Aug 01, 2019 at 10:17:30AM +, Rantala, Tommi T. (Nokia - > FI/Espoo) wrote: > > Hi, > > > > This tipc patch added in 4.14.132 is triggering a crash for me, > > revert > > fixes it. > > > > Anyone have ideas if some other commits missing in 4.14.x to make > > this > > work...? > > Do you also hav a problem with 4.19.y? How about 5.2.y? If not, can > you do 'git bisect' to find the patch that fixes the issue? > > thanks, > > greg k-h Hi, please pick this to 4.14.y and 4.19.y, tested that it fixes the crash in both: commit 5684abf7020dfc5f0b6ba1d68eda3663871fce52 Author: Xin Long Date: Mon Jun 17 21:34:13 2019 +0800 ip_tunnel: allow not to count pkts on tstats by setting skb's dev to NULL For 5.2.y nothing is needed, these commits were in v5.2-rc6 already. -Tommi
Re: [PATCH 4.14 43/43] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
On Thu, Aug 01, 2019 at 10:17:30AM +, Rantala, Tommi T. (Nokia - FI/Espoo) wrote: > On Tue, 2019-07-02 at 10:02 +0200, Greg Kroah-Hartman wrote: > > From: Xin Long > > > > commit c3bcde026684c62d7a2b6f626dc7cf763833875c upstream. > > > > udp_tunnel(6)_xmit_skb() called by tipc_udp_xmit() expects a tunnel > > device > > to count packets on dev->tstats, a perpcu variable. However, TIPC is > > using > > udp tunnel with no tunnel device, and pass the lower dev, like veth > > device > > that only initializes dev->lstats(a perpcu variable) when creating > > it. > > Hi, > > This tipc patch added in 4.14.132 is triggering a crash for me, revert > fixes it. > > Anyone have ideas if some other commits missing in 4.14.x to make this > work...? Do you also hav a problem with 4.19.y? How about 5.2.y? If not, can you do 'git bisect' to find the patch that fixes the issue? thanks, greg k-h
Re: [PATCH 4.14 43/43] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
On Tue, 2019-07-02 at 10:02 +0200, Greg Kroah-Hartman wrote: > From: Xin Long > > commit c3bcde026684c62d7a2b6f626dc7cf763833875c upstream. > > udp_tunnel(6)_xmit_skb() called by tipc_udp_xmit() expects a tunnel > device > to count packets on dev->tstats, a perpcu variable. However, TIPC is > using > udp tunnel with no tunnel device, and pass the lower dev, like veth > device > that only initializes dev->lstats(a perpcu variable) when creating > it. Hi, This tipc patch added in 4.14.132 is triggering a crash for me, revert fixes it. Anyone have ideas if some other commits missing in 4.14.x to make this work...? # modprobe tipc # tipc node set addr 1.1.2 # tipc bearer enable media udp name UDP1 localip 192.168.1.15 [ 143.105529] Own node address <1.1.2>, network identity 4711 [ 172.087098] BUG: unable to handle kernel NULL pointer dereference at 04f0 [ 172.088375] IP: iptunnel_xmit+0x15e/0x1e0 [ 172.089072] PGD 800231306067 P4D 800231306067 PUD 2356e1067 PMD 0 [ 172.090094] Oops: [#1] SMP PTI [ 172.090610] Modules linked in: tipc ip6_udp_tunnel udp_tunnel isofs kvm_intel kvm irqbypass sch_fq_codel pcbc aesni_intel aes_x86_64 crypto_simd cryptd glue_helper ata_piix dm_mirror dm_region_hash dm_log dm_mod dax autofs4 [ 172.093293] CPU: 1 PID: 747 Comm: tipc Not tainted 4.14.134-1.x86_64 #1 [ 172.094448] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-2.fc30 04/01/2014 [ 172.095703] task: 8b99f12c task.stack: 9ab481198000 [ 172.096731] RIP: 0010:iptunnel_xmit+0x15e/0x1e0 [ 172.097460] RSP: 0018:9ab48119ba00 EFLAGS: 00010202 [ 172.098214] RAX: RBX: bf4d8140 RCX: 008c [ 172.099320] RDX: 0001 RSI: fe01 RDI: be944d62 [ 172.100392] RBP: 8b99f1e7ed00 R08: 8b99ffc64520 R09: [ 172.101451] R10: 00023426d000 R11: 0002 R12: [ 172.102607] R13: 0040 R14: R15: 8b99f426e0e8 [ 172.103728] FS: 7efc82b96800() GS:8b99ffc4() knlGS: [ 172.104976] CS: 0010 DS: ES: CR0: 80050033 [ 172.105821] CR2: 04f0 CR3: 000234250001 CR4: 003606e0 [ 172.106981] DR0: DR1: DR2: [ 172.108120] DR3: DR6: fffe0ff0 DR7: 0400 [ 172.109386] Call Trace: [ 172.109808] tipc_udp_xmit.isra.18+0x1a7/0x1c0 [tipc] [ 172.110687] ? __internal_add_timer+0x1a/0x50 [ 172.111369] ? __skb_clone+0x29/0x130 [ 172.111999] tipc_bearer_xmit_skb+0x4d/0x80 [tipc] [ 172.112845] tipc_enable_bearer+0x2b9/0x3c0 [tipc] [ 172.113637] ? __nla_put+0xc/0x20 [ 172.114213] tipc_nl_bearer_enable+0xca/0x100 [tipc] [ 172.114952] genl_family_rcv_msg+0x190/0x390 [ 172.115748] genl_rcv_msg+0x47/0x90 [ 172.116287] ? __alloc_skb+0x72/0x1b0 [ 172.116898] ? genl_family_rcv_msg+0x390/0x390 [ 172.117669] netlink_rcv_skb+0x3d/0x100 [ 172.118361] genl_rcv+0x24/0x40 [ 172.119005] netlink_unicast+0x16d/0x230 [ 172.119777] netlink_sendmsg+0x1ae/0x3c0 [ 172.120525] SYSC_sendto+0xe6/0x140 [ 172.121248] ? SYSC_getsockname+0x81/0xa0 [ 172.121989] ? sock_alloc_file+0x97/0x120 [ 172.122645] ? sock_map_fd+0x3d/0x60 [ 172.123278] do_syscall_64+0x74/0x190 [ 172.123911] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 [ 172.124716] RIP: 0033:0x7efc82d6ac6b [ 172.125368] RSP: 002b:7fff40411ae8 EFLAGS: 0246 ORIG_RAX: 002c [ 172.126486] RAX: ffda RBX: 01dfca20 RCX: 7efc82d6ac6b [ 172.127632] RDX: 0054 RSI: 7fff40411b60 RDI: 0003 [ 172.128765] RBP: 7fff40411b50 R08: 7efc82e36000 R09: 000c [ 172.129793] R10: R11: 0246 R12: 7fff40411b60 [ 172.130799] R13: 7fff40412d10 R14: 0040bb44 R15: [ 172.131868] Code: 01 00 00 00 85 d2 0f 44 d0 e8 1f f3 fa ff 48 8b 74 24 08 4c 89 fa 48 89 df e8 9f 94 fb ff 83 e0 fd 75 35 8b 4c 24 1c 85 c9 7e 2b <49> 8b 84 24 f0 04 00 00 65 48 03 05 aa 29 68 41 48 83 40 10 01 [ 172.134773] RIP: iptunnel_xmit+0x15e/0x1e0 RSP: 9ab48119ba00 [ 172.135697] CR2: 04f0 [ 172.136305] ---[ end trace 27f7522ade26797f ]--- > Later iptunnel_xmit_stats() called by ip(6)tunnel_xmit() thinks the > dev as > a tunnel device, and uses dev->tstats instead of dev->lstats. tstats' > each > pointer points to a bigger struct than lstats, so when tstats- > >tx_bytes is > increased, other percpu variable's members could be overwritten. > > syzbot has reported quite a few crashes due to fib_nh_common percpu > member > 'nhc_pcpu_rth_output' overwritten, call traces are like: > > BUG: KASAN: slab-out-of-bounds in rt_cache_valid+0x158/0x190 > net/ipv4/route.c:1556 > rt_cache_valid+0x158/0x190 net/ipv4/route.c:1556 > __mkroute_output net/ipv4/route.c:2332 [inline] >
[PATCH 4.14 43/43] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
From: Xin Long
commit c3bcde026684c62d7a2b6f626dc7cf763833875c upstream.
udp_tunnel(6)_xmit_skb() called by tipc_udp_xmit() expects a tunnel device
to count packets on dev->tstats, a perpcu variable. However, TIPC is using
udp tunnel with no tunnel device, and pass the lower dev, like veth device
that only initializes dev->lstats(a perpcu variable) when creating it.
Later iptunnel_xmit_stats() called by ip(6)tunnel_xmit() thinks the dev as
a tunnel device, and uses dev->tstats instead of dev->lstats. tstats' each
pointer points to a bigger struct than lstats, so when tstats->tx_bytes is
increased, other percpu variable's members could be overwritten.
syzbot has reported quite a few crashes due to fib_nh_common percpu member
'nhc_pcpu_rth_output' overwritten, call traces are like:
BUG: KASAN: slab-out-of-bounds in rt_cache_valid+0x158/0x190
net/ipv4/route.c:1556
rt_cache_valid+0x158/0x190 net/ipv4/route.c:1556
__mkroute_output net/ipv4/route.c:2332 [inline]
ip_route_output_key_hash_rcu+0x819/0x2d50 net/ipv4/route.c:2564
ip_route_output_key_hash+0x1ef/0x360 net/ipv4/route.c:2393
__ip_route_output_key include/net/route.h:125 [inline]
ip_route_output_flow+0x28/0xc0 net/ipv4/route.c:2651
ip_route_output_key include/net/route.h:135 [inline]
...
or:
kasan: GPF could be caused by NULL-ptr deref or user memory access
RIP: 0010:dst_dev_put+0x24/0x290 net/core/dst.c:168
rt_fibinfo_free_cpus net/ipv4/fib_semantics.c:200 [inline]
free_fib_info_rcu+0x2e1/0x490 net/ipv4/fib_semantics.c:217
__rcu_reclaim kernel/rcu/rcu.h:240 [inline]
rcu_do_batch kernel/rcu/tree.c:2437 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2716 [inline]
rcu_process_callbacks+0x100a/0x1ac0 kernel/rcu/tree.c:2697
...
The issue exists since tunnel stats update is moved to iptunnel_xmit by
Commit 039f50629b7f ("ip_tunnel: Move stats update to iptunnel_xmit()"),
and here to fix it by passing a NULL tunnel dev to udp_tunnel(6)_xmit_skb
so that the packets counting won't happen on dev->tstats.
Reported-by: syzbot+9d4c12bfd45a58738...@syzkaller.appspotmail.com
Reported-by: syzbot+a9e23ea2aa21044c2...@syzkaller.appspotmail.com
Reported-by: syzbot+c4c4b2bb358bb936a...@syzkaller.appspotmail.com
Reported-by: syzbot+0290d2290a607e035...@syzkaller.appspotmail.com
Reported-by: syzbot+a43d8d4e7e8a7a9e1...@syzkaller.appspotmail.com
Reported-by: syzbot+a47c5f4c6c00fc1ed...@syzkaller.appspotmail.com
Fixes: 039f50629b7f ("ip_tunnel: Move stats update to iptunnel_xmit()")
Signed-off-by: Xin Long
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
---
net/tipc/udp_media.c |8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -174,7 +174,6 @@ static int tipc_udp_xmit(struct net *net
goto tx_error;
}
- skb->dev = rt->dst.dev;
ttl = ip4_dst_hoplimit(>dst);
udp_tunnel_xmit_skb(rt, ub->ubsock->sk, skb, src->ipv4.s_addr,
dst->ipv4.s_addr, 0, ttl, 0, src->port,
@@ -193,10 +192,9 @@ static int tipc_udp_xmit(struct net *net
if (err)
goto tx_error;
ttl = ip6_dst_hoplimit(ndst);
- err = udp_tunnel6_xmit_skb(ndst, ub->ubsock->sk, skb,
- ndst->dev, >ipv6,
- >ipv6, 0, ttl, 0, src->port,
- dst->port, false);
+ err = udp_tunnel6_xmit_skb(ndst, ub->ubsock->sk, skb, NULL,
+ >ipv6, >ipv6, 0, ttl, 0,
+ src->port, dst->port, false);
#endif
}
return err;
