Re: [4.9.10] ip_route_me_harder() reading off-slab

[Pablo Neira Ayuso]

On Mon, Feb 27, 2017 at 10:41:48PM +0800, Daniel J Blueman wrote:
> On 17 February 2017 at 15:39, Florian Westphal  wrote:
> > Daniel J Blueman  wrote:
> >
> > [ CC nf-devel, pablo ]
> >
> >> When booting a VM in libvirt/KVM attached to a local bridge and KASAN
> >> enabled on 4.9.10, we see a stream of KASAN warnings about off-slab
> >> access [1].
> >>
> >> Let me know if you'd like more debug.
> >
> > Does this patch help?
> >
> > Subject: [PATCH nf] netfilter: use skb_to_full_sk in ip_route_me_harder
> >
> > inet_sk(skb->sk) is illegal in case skb is attached to request socket.
> >
> > Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets 
> > instead of listener")
> > Reported by: Daniel J Blueman 
> > Signed-off-by: Florian Westphal 
[...]
> Apologies for the delays; this also addresses the issue just fine.
> 
> Tested-by: Daniel J Blueman 

Applied, thanks for testing.

Re: [4.9.10] ip_route_me_harder() reading off-slab

[Daniel J Blueman]

On 17 February 2017 at 15:39, Florian Westphal  wrote:
> Daniel J Blueman  wrote:
>
> [ CC nf-devel, pablo ]
>
>> When booting a VM in libvirt/KVM attached to a local bridge and KASAN
>> enabled on 4.9.10, we see a stream of KASAN warnings about off-slab
>> access [1].
>>
>> Let me know if you'd like more debug.
>
> Does this patch help?
>
> Subject: [PATCH nf] netfilter: use skb_to_full_sk in ip_route_me_harder
>
> inet_sk(skb->sk) is illegal in case skb is attached to request socket.
>
> Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead 
> of listener")
> Reported by: Daniel J Blueman 
> Signed-off-by: Florian Westphal 
> ---
>  net/ipv4/netfilter.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
> index b3cc1335adbc..c0cc6aa8cfaa 100644
> --- a/net/ipv4/netfilter.c
> +++ b/net/ipv4/netfilter.c
> @@ -23,7 +23,8 @@ int ip_route_me_harder(struct net *net, struct sk_buff 
> *skb, unsigned int addr_t
> struct rtable *rt;
> struct flowi4 fl4 = {};
> __be32 saddr = iph->saddr;
> -   __u8 flags = skb->sk ? inet_sk_flowi_flags(skb->sk) : 0;
> +   const struct sock *sk = skb_to_full_sk(skb);
> +   __u8 flags = sk ? inet_sk_flowi_flags(sk) : 0;
> struct net_device *dev = skb_dst(skb)->dev;
> unsigned int hh_len;
>
> @@ -40,7 +41,7 @@ int ip_route_me_harder(struct net *net, struct sk_buff 
> *skb, unsigned int addr_t
> fl4.daddr = iph->daddr;
> fl4.saddr = saddr;
> fl4.flowi4_tos = RT_TOS(iph->tos);
> -   fl4.flowi4_oif = skb->sk ? skb->sk->sk_bound_dev_if : 0;
> +   fl4.flowi4_oif = sk ? sk->sk_bound_dev_if : 0;
> if (!fl4.flowi4_oif)
> fl4.flowi4_oif = l3mdev_master_ifindex(dev);
> fl4.flowi4_mark = skb->mark;
> @@ -61,7 +62,7 @@ int ip_route_me_harder(struct net *net, struct sk_buff 
> *skb, unsigned int addr_t
> xfrm_decode_session(skb, flowi4_to_flowi(), AF_INET) == 0) {
> struct dst_entry *dst = skb_dst(skb);
> skb_dst_set(skb, NULL);
> -   dst = xfrm_lookup(net, dst, flowi4_to_flowi(), skb->sk, 
> 0);
> +   dst = xfrm_lookup(net, dst, flowi4_to_flowi(), sk, 0);
> if (IS_ERR(dst))
> return PTR_ERR(dst);
> skb_dst_set(skb, dst);

Apologies for the delays; this also addresses the issue just fine.

Tested-by: Daniel J Blueman 

Dan
-- 
Daniel J Blueman

Re: [4.9.10] ip_route_me_harder() reading off-slab

[Daniel J Blueman]

On 17 February 2017 at 13:36, Eric Dumazet  wrote:
> On Fri, 2017-02-17 at 12:36 +0800, Daniel J Blueman wrote:
>> When booting a VM in libvirt/KVM attached to a local bridge and KASAN
>> enabled on 4.9.10, we see a stream of KASAN warnings about off-slab
>> access [1].
>>
>> Let me know if you'd like more debug.
>
> Could you try the following patch ?
>
> Thanks !
>
> diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
> index 
> b3cc1335adbc1a20dcd225d0501b0a286d27e3c8..18839e59da849f0988924bcbc9873965a3681eb0
>  100644
> --- a/net/ipv4/netfilter.c
> +++ b/net/ipv4/netfilter.c
> @@ -23,7 +23,8 @@ int ip_route_me_harder(struct net *net, struct sk_buff 
> *skb, unsigned int addr_t
> struct rtable *rt;
> struct flowi4 fl4 = {};
> __be32 saddr = iph->saddr;
> -   __u8 flags = skb->sk ? inet_sk_flowi_flags(skb->sk) : 0;
> +   struct sock *sk = skb->sk;
> +   __u8 flags = sk && sk_fullsock(sk) ? inet_sk_flowi_flags(sk) : 0;
> struct net_device *dev = skb_dst(skb)->dev;
> unsigned int hh_len;
>
> @@ -40,7 +41,7 @@ int ip_route_me_harder(struct net *net, struct sk_buff 
> *skb, unsigned int addr_t
> fl4.daddr = iph->daddr;
> fl4.saddr = saddr;
> fl4.flowi4_tos = RT_TOS(iph->tos);
> -   fl4.flowi4_oif = skb->sk ? skb->sk->sk_bound_dev_if : 0;
> +   fl4.flowi4_oif = sk ? sk->sk_bound_dev_if : 0;
> if (!fl4.flowi4_oif)
> fl4.flowi4_oif = l3mdev_master_ifindex(dev);
> fl4.flowi4_mark = skb->mark;
> @@ -61,7 +62,7 @@ int ip_route_me_harder(struct net *net, struct sk_buff 
> *skb, unsigned int addr_t
> xfrm_decode_session(skb, flowi4_to_flowi(), AF_INET) == 0) {
> struct dst_entry *dst = skb_dst(skb);
> skb_dst_set(skb, NULL);
> -   dst = xfrm_lookup(net, dst, flowi4_to_flowi(), skb->sk, 
> 0);
> +   dst = xfrm_lookup(net, dst, flowi4_to_flowi(), sk, 0);
> if (IS_ERR(dst))
> return PTR_ERR(dst);
> skb_dst_set(skb, dst);

Fine work! This nicely resolves the issue. I'll test Florian's
proposed fix also.

Tested-by: Daniel J Blueman 

Thanks,
  Dan
-- 
Daniel J Blueman

Re: [4.9.10] ip_route_me_harder() reading off-slab

[Florian Westphal]

Daniel J Blueman  wrote:

[ CC nf-devel, pablo ]

> When booting a VM in libvirt/KVM attached to a local bridge and KASAN
> enabled on 4.9.10, we see a stream of KASAN warnings about off-slab
> access [1].
> 
> Let me know if you'd like more debug.

Does this patch help?

Subject: [PATCH nf] netfilter: use skb_to_full_sk in ip_route_me_harder

inet_sk(skb->sk) is illegal in case skb is attached to request socket.

Fixes: ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of 
listener")
Reported by: Daniel J Blueman 
Signed-off-by: Florian Westphal 
---
 net/ipv4/netfilter.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index b3cc1335adbc..c0cc6aa8cfaa 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c
@@ -23,7 +23,8 @@ int ip_route_me_harder(struct net *net, struct sk_buff *skb, 
unsigned int addr_t
struct rtable *rt;
struct flowi4 fl4 = {};
__be32 saddr = iph->saddr;
-   __u8 flags = skb->sk ? inet_sk_flowi_flags(skb->sk) : 0;
+   const struct sock *sk = skb_to_full_sk(skb);
+   __u8 flags = sk ? inet_sk_flowi_flags(sk) : 0;
struct net_device *dev = skb_dst(skb)->dev;
unsigned int hh_len;
 
@@ -40,7 +41,7 @@ int ip_route_me_harder(struct net *net, struct sk_buff *skb, 
unsigned int addr_t
fl4.daddr = iph->daddr;
fl4.saddr = saddr;
fl4.flowi4_tos = RT_TOS(iph->tos);
-   fl4.flowi4_oif = skb->sk ? skb->sk->sk_bound_dev_if : 0;
+   fl4.flowi4_oif = sk ? sk->sk_bound_dev_if : 0;
if (!fl4.flowi4_oif)
fl4.flowi4_oif = l3mdev_master_ifindex(dev);
fl4.flowi4_mark = skb->mark;
@@ -61,7 +62,7 @@ int ip_route_me_harder(struct net *net, struct sk_buff *skb, 
unsigned int addr_t
xfrm_decode_session(skb, flowi4_to_flowi(), AF_INET) == 0) {
struct dst_entry *dst = skb_dst(skb);
skb_dst_set(skb, NULL);
-   dst = xfrm_lookup(net, dst, flowi4_to_flowi(), skb->sk, 0);
+   dst = xfrm_lookup(net, dst, flowi4_to_flowi(), sk, 0);
if (IS_ERR(dst))
return PTR_ERR(dst);
skb_dst_set(skb, dst);
-- 
2.10.2

Re: [4.9.10] ip_route_me_harder() reading off-slab

[Willy Tarreau]

On Fri, Feb 17, 2017 at 01:34:11PM +0800, Daniel J Blueman wrote:
> When booting a VM in libvirt/KVM attached to a local bridge and KASAN
> enabled on 4.9.10, we see a stream of KASAN warnings about off-slab
> access [1].

Did it start to appear with 4.9.10 or is 4.9.10 the first 4.9 kernel
you tried (ie is it a regression between earlier kernels and 4.9 or
a recent faulty stable backport into 4.9) ?

Willy


> Let me know if you'd like more debug.
> 
> Thanks,
>   Daniel
> 
> -- [1]
> 
> [  473.579567] BUG: KASAN: slab-out-of-bounds in
> ip_route_me_harder+0xbd5/0xf20 at addr 8801e1eb28a8
> [  473.579577] Read of size 1 by task vcselab/10339
> [  473.579590] CPU: 1 PID: 10339 Comm: vcselab Tainted: GB  4.9.10-debug+ 
> #2
> [  473.579596] Hardware name: Dell Inc. XPS 13 9360/0T3FTF, BIOS 1.3.2
> 01/18/2017
> [  473.579602]  880236086ed0 8aed83a1 8802324fe6c0
> 8801e1eb26f8
> [  473.579626]  880236086ef8 8a849521 880236086f90
> 8801e1eb26f0
> [  473.579645]  8802324fe6c0 880236086f80 8a8497ba
> 8a848b2d
> [  473.579662] Call Trace:
> [  473.579667]  
> [  473.579685]  [] dump_stack+0x85/0xc4
> [  473.579698]  [] kasan_object_err+0x21/0x70
> [  473.579709]  [] kasan_report_error+0x1fa/0x500
> [  473.579720]  [] ? kasan_kmalloc+0xad/0xe0
> [  473.579737]  [] __asan_report_load1_noabort+0x61/0x70
> [  473.579749]  [] ? ip_route_me_harder+0xbd5/0xf20
> [  473.579759]  [] ip_route_me_harder+0xbd5/0xf20
> [  473.579772]  [] ? nf_ip_saveroute+0x320/0x320
> [  473.579785]  [] ? entry_SYSCALL_64_fastpath+0x23/0xc6
> [  473.579801]  [] iptable_mangle_hook+0x3da/0x5f0
> [iptable_mangle]
> [  473.579814]  [] nf_iterate+0x110/0x2d0
> [  473.579826]  [] nf_hook_slow+0xf6/0x1b0
> [  473.579839]  [] ? nf_iterate+0x2d0/0x2d0
> [  473.579850]  [] ? __ip_local_out+0x28b/0x720
> [  473.579860]  [] __ip_local_out+0x366/0x720
> [  473.579869]  [] ? __ip_local_out+0x28b/0x720
> [  473.579879]  [] ? ip_finish_output+0x9b0/0x9b0
> [  473.579894]  [] ?
> __ip_flush_pending_frames.isra.43+0x2e0/0x2e0
> [  473.579905]  [] ip_local_out+0x1e/0x130
> [  473.579915]  [] ip_build_and_send_pkt+0x54d/0xad0
> [  473.579927]  [] tcp_v4_send_synack+0x184/0x290
> [  473.579937]  [] ? tcp_v4_send_check+0x90/0x90
> [  473.579950]  [] ? inet_ehash_insert+0x407/0x910
> [  473.579965]  [] tcp_conn_request+0x1f5b/0x2a20
> [  473.579977]  [] ? tcp_check_space+0x580/0x580
> [  473.579991]  [] ? default_wake_function+0x35/0x50
> [  473.580007]  [] ? debug_check_no_locks_freed+0x290/0x290
> [  473.580018]  [] ? debug_check_no_locks_freed+0x290/0x290
> [  473.580031]  [] ? ipt_do_table+0xb14/0x1ac0 [ip_tables]
> [  473.580041]  [] ? trace_hardirqs_on+0xd/0x10
> [  473.580055]  [] ? __local_bh_enable_ip+0x70/0xc0
> [  473.580067]  [] tcp_v4_conn_request+0x134/0x1e0
> [  473.580079]  [] tcp_v6_conn_request+0x1b8/0x230
> [  473.580089]  [] tcp_rcv_state_process+0x61d/0x41a0
> [  473.580101]  [] ? sk_filter_trim_cap+0x2a7/0x6a0
> [  473.580114]  [] ? tcp_finish_connect+0x600/0x600
> [  473.580125]  [] ? sk_filter_trim_cap+0x2c6/0x6a0
> [  473.580135]  [] ? sk_filter_trim_cap+0xf8/0x6a0
> [  473.580145]  [] ? tcp_md5_do_lookup+0x4a/0x190
> [  473.580157]  [] ? sk_filter_is_valid_access+0x60/0x60
> [  473.580170]  [] ? tcp_v4_inbound_md5_hash+0x139/0x3bb
> [  473.580180]  [] ? ncsi_start_dev+0x111/0x111
> [  473.580190]  [] tcp_v4_do_rcv+0x2c8/0x8c0
> [  473.580201]  [] tcp_v4_rcv+0x23a8/0x2fc0
> [  473.580214]  [] ? ipv4_confirm+0x117/0x3d0
> [nf_conntrack_ipv4]
> [  473.580228]  [] ip_local_deliver_finish+0x2b9/0x970
> [  473.580241]  [] ? ip_local_deliver_finish+0x12a/0x970
> [  473.580251]  [] ip_local_deliver+0x1b4/0x460
> [  473.580259]  [] ? ip_local_deliver+0x202/0x460
> [  473.580267]  [] ? ip_call_ra_chain+0x510/0x510
> [  473.580280]  [] ? iptable_nat_ipv4_in+0x15/0x20
> [iptable_nat]
> [  473.580290]  [] ? nf_iterate+0x92/0x2d0
> [  473.580302]  [] ? ip_rcv_finish+0x18e0/0x18e0
> [  473.580313]  [] ? nf_hook_slow+0xf6/0x1b0
> [  473.580323]  [] ip_rcv_finish+0x655/0x18e0
> [  473.580331]  [] ? ip_rcv+0x9db/0x1280
> [  473.580341]  [] ip_rcv+0x843/0x1280
> [  473.580352]  [] ? ip_rcv+0x8d3/0x1280
> [  473.580363]  [] ? __enqueue_entity+0x139/0x230
> [  473.580373]  [] ? ip_local_deliver+0x460/0x460
> [  473.580382]  [] ? inet_del_offload+0x40/0x40
> [  473.580393]  [] ? ip_local_deliver+0x460/0x460
> [  473.580407]  [] __netif_receive_skb_core+0x15d9/0x2c90
> [  473.580419]  [] ? enqueue_task_fair+0x261/0x2980
> [  473.580428]  [] ? debug_check_no_locks_freed+0x290/0x290
> [  473.580439]  [] ? netif_wake_subqueue+0x1c0/0x1c0
> [  473.580453]  [] __netif_receive_skb+0x24/0x150
> [  473.580468]  [] process_backlog+0xd7/0x610
> [  473.580477]  [] ? process_backlog+0x204/0x610
> [  473.580487]  [] ? swake_up+0x3b/0x60
> [  473.580498]  [] net_rx_action+0x731/0xe60
> [  473.580510]  [] ? sk_busy_loop+0xae0/0xae0
> [  473.580527]  [] ? clockevents_program_event+0x1cf/0x300
> [