* [PATCH net 0/3] net: fix quite a few dst_cache crashes reported by syzbot
@ 2019-06-17 13:34 Xin Long
2019-06-17 13:34 ` [PATCH net 1/3] ip_tunnel: allow not to count pkts on tstats by setting skb's dev to NULL Xin Long
2019-06-19 0:49 ` [PATCH net 0/3] net: fix quite a few dst_cache crashes reported by syzbot David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Xin Long @ 2019-06-17 13:34 UTC (permalink / raw)
To: network dev
Cc: davem, Jon Maloy, Ying Xue, tipc-discussion,
Marcelo Ricardo Leitner, Neil Horman, Su Yanjun, David Ahern,
syzkaller-bugs, Dmitry Vyukov, Pravin B Shelar
There are two kinds of crashes reported many times by syzbot with no
reproducer. 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
<IRQ>
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
...
They were caused by the fib_nh_common percpu member 'nhc_pcpu_rth_output'
overwritten by another percpu variable 'dev->tstats' access overflow in
tipc udp media xmit path when counting packets on a non tunnel device.
The fix is to make udp tunnel work with no tunnel device by allowing not
to count packets on the tstats when the tunnel dev is NULL in Patches 1/3
and 2/3, then pass a NULL tunnel dev in tipc_udp_tunnel() in Patch 3/3.
Xin Long (3):
ip_tunnel: allow not to count pkts on tstats by setting skb's dev to
NULL
ip6_tunnel: allow not to count pkts on tstats by passing dev as NULL
tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
include/net/ip6_tunnel.h | 9 ++++++---
net/ipv4/ip_tunnel_core.c | 9 ++++++---
net/tipc/udp_media.c | 8 +++-----
3 files changed, 15 insertions(+), 11 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net 1/3] ip_tunnel: allow not to count pkts on tstats by setting skb's dev to NULL
2019-06-17 13:34 [PATCH net 0/3] net: fix quite a few dst_cache crashes reported by syzbot Xin Long
@ 2019-06-17 13:34 ` Xin Long
2019-06-17 13:34 ` [PATCH net 2/3] ip6_tunnel: allow not to count pkts on tstats by passing dev as NULL Xin Long
2019-06-19 0:49 ` [PATCH net 0/3] net: fix quite a few dst_cache crashes reported by syzbot David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Xin Long @ 2019-06-17 13:34 UTC (permalink / raw)
To: network dev
Cc: davem, Jon Maloy, Ying Xue, tipc-discussion,
Marcelo Ricardo Leitner, Neil Horman, Su Yanjun, David Ahern,
syzkaller-bugs, Dmitry Vyukov, Pravin B Shelar
iptunnel_xmit() works as a common function, also used by a udp tunnel
which doesn't have to have a tunnel device, like how TIPC works with
udp media.
In these cases, we should allow not to count pkts on dev's tstats, so
that udp tunnel can work with no tunnel device safely.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/ipv4/ip_tunnel_core.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index 30c1c26..5073e3c 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -89,9 +89,12 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
__ip_select_ident(net, iph, skb_shinfo(skb)->gso_segs ?: 1);
err = ip_local_out(net, sk, skb);
- if (unlikely(net_xmit_eval(err)))
- pkt_len = 0;
- iptunnel_xmit_stats(dev, pkt_len);
+
+ if (dev) {
+ if (unlikely(net_xmit_eval(err)))
+ pkt_len = 0;
+ iptunnel_xmit_stats(dev, pkt_len);
+ }
}
EXPORT_SYMBOL_GPL(iptunnel_xmit);
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net 2/3] ip6_tunnel: allow not to count pkts on tstats by passing dev as NULL
2019-06-17 13:34 ` [PATCH net 1/3] ip_tunnel: allow not to count pkts on tstats by setting skb's dev to NULL Xin Long
@ 2019-06-17 13:34 ` Xin Long
2019-06-17 13:34 ` [PATCH net 3/3] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb Xin Long
0 siblings, 1 reply; 5+ messages in thread
From: Xin Long @ 2019-06-17 13:34 UTC (permalink / raw)
To: network dev
Cc: davem, Jon Maloy, Ying Xue, tipc-discussion,
Marcelo Ricardo Leitner, Neil Horman, Su Yanjun, David Ahern,
syzkaller-bugs, Dmitry Vyukov, Pravin B Shelar
A similar fix to Patch "ip_tunnel: allow not to count pkts on tstats by
setting skb's dev to NULL" is also needed by ip6_tunnel.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/ip6_tunnel.h | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index 69b4bcf..028eaea 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -158,9 +158,12 @@ static inline void ip6tunnel_xmit(struct sock *sk, struct sk_buff *skb,
memset(skb->cb, 0, sizeof(struct inet6_skb_parm));
pkt_len = skb->len - skb_inner_network_offset(skb);
err = ip6_local_out(dev_net(skb_dst(skb)->dev), sk, skb);
- if (unlikely(net_xmit_eval(err)))
- pkt_len = -1;
- iptunnel_xmit_stats(dev, pkt_len);
+
+ if (dev) {
+ if (unlikely(net_xmit_eval(err)))
+ pkt_len = -1;
+ iptunnel_xmit_stats(dev, pkt_len);
+ }
}
#endif
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net 3/3] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb
2019-06-17 13:34 ` [PATCH net 2/3] ip6_tunnel: allow not to count pkts on tstats by passing dev as NULL Xin Long
@ 2019-06-17 13:34 ` Xin Long
0 siblings, 0 replies; 5+ messages in thread
From: Xin Long @ 2019-06-17 13:34 UTC (permalink / raw)
To: network dev
Cc: davem, Jon Maloy, Ying Xue, tipc-discussion,
Marcelo Ricardo Leitner, Neil Horman, Su Yanjun, David Ahern,
syzkaller-bugs, Dmitry Vyukov, Pravin B Shelar
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
<IRQ>
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+9d4c12bfd45a58738d0a@syzkaller.appspotmail.com
Reported-by: syzbot+a9e23ea2aa21044c2798@syzkaller.appspotmail.com
Reported-by: syzbot+c4c4b2bb358bb936ad7e@syzkaller.appspotmail.com
Reported-by: syzbot+0290d2290a607e035ba1@syzkaller.appspotmail.com
Reported-by: syzbot+a43d8d4e7e8a7a9e149e@syzkaller.appspotmail.com
Reported-by: syzbot+a47c5f4c6c00fc1ed16e@syzkaller.appspotmail.com
Fixes: 039f50629b7f ("ip_tunnel: Move stats update to iptunnel_xmit()")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/udp_media.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 7fc02d8..1405ccc 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -176,7 +176,6 @@ static int tipc_udp_xmit(struct net *net, struct sk_buff *skb,
goto tx_error;
}
- skb->dev = rt->dst.dev;
ttl = ip4_dst_hoplimit(&rt->dst);
udp_tunnel_xmit_skb(rt, ub->ubsock->sk, skb, src->ipv4.s_addr,
dst->ipv4.s_addr, 0, ttl, 0, src->port,
@@ -195,10 +194,9 @@ static int tipc_udp_xmit(struct net *net, struct sk_buff *skb,
if (err)
goto tx_error;
ttl = ip6_dst_hoplimit(ndst);
- err = udp_tunnel6_xmit_skb(ndst, ub->ubsock->sk, skb,
- ndst->dev, &src->ipv6,
- &dst->ipv6, 0, ttl, 0, src->port,
- dst->port, false);
+ err = udp_tunnel6_xmit_skb(ndst, ub->ubsock->sk, skb, NULL,
+ &src->ipv6, &dst->ipv6, 0, ttl, 0,
+ src->port, dst->port, false);
#endif
}
return err;
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net 0/3] net: fix quite a few dst_cache crashes reported by syzbot
2019-06-17 13:34 [PATCH net 0/3] net: fix quite a few dst_cache crashes reported by syzbot Xin Long
2019-06-17 13:34 ` [PATCH net 1/3] ip_tunnel: allow not to count pkts on tstats by setting skb's dev to NULL Xin Long
@ 2019-06-19 0:49 ` David Miller
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2019-06-19 0:49 UTC (permalink / raw)
To: lucien.xin
Cc: netdev, jon.maloy, ying.xue, tipc-discussion, marcelo.leitner,
nhorman, suyj.fnst, dsahern, syzkaller-bugs, dvyukov, pshelar
From: Xin Long <lucien.xin@gmail.com>
Date: Mon, 17 Jun 2019 21:34:12 +0800
> There are two kinds of crashes reported many times by syzbot with no
> reproducer. Call Traces are like:
...
> They were caused by the fib_nh_common percpu member 'nhc_pcpu_rth_output'
> overwritten by another percpu variable 'dev->tstats' access overflow in
> tipc udp media xmit path when counting packets on a non tunnel device.
>
> The fix is to make udp tunnel work with no tunnel device by allowing not
> to count packets on the tstats when the tunnel dev is NULL in Patches 1/3
> and 2/3, then pass a NULL tunnel dev in tipc_udp_tunnel() in Patch 3/3.
Series applied and queued up for -stable.
Thanks for putting all of those syzbot reported by tags in there.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-19 0:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17 13:34 [PATCH net 0/3] net: fix quite a few dst_cache crashes reported by syzbot Xin Long
2019-06-17 13:34 ` [PATCH net 1/3] ip_tunnel: allow not to count pkts on tstats by setting skb's dev to NULL Xin Long
2019-06-17 13:34 ` [PATCH net 2/3] ip6_tunnel: allow not to count pkts on tstats by passing dev as NULL Xin Long
2019-06-17 13:34 ` [PATCH net 3/3] tipc: pass tunnel dev as NULL to udp_tunnel(6)_xmit_skb Xin Long
2019-06-19 0:49 ` [PATCH net 0/3] net: fix quite a few dst_cache crashes reported by syzbot David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).