Re: [PATCH net] dccp: fix use-after-free in dccp_feat_activate_values

[David Miller]

From: Eric Dumazet 
Date: Sun, 05 Mar 2017 10:52:16 -0800

> From: Eric Dumazet 
> 
> Dmitry reported crashes in DCCP stack [1]
> 
> Problem here is that when I got rid of listener spinlock, I missed the
> fact that DCCP stores a complex state in struct dccp_request_sock,
> while TCP does not.
> 
> Since multiple cpus could access it at the same time, we need to add
> protection.
 ...
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Signed-off-by: Eric Dumazet 
> Reported-by: Dmitry Vyukov 
> Tested-by: Dmitry Vyukov 

Applied and queued up for -stable.

Re: [PATCH net] dccp: fix use-after-free in dccp_feat_activate_values

[Cong Wang]

On Sun, Mar 5, 2017 at 10:42 PM, Eric Dumazet  wrote:
> On Sun, 2017-03-05 at 21:38 -0800, Cong Wang wrote:
>
>> Do you really want to disable BH again here?
>>
>> dccp_check_req() should be always called on RX path where BH
>> is already disabled and BH can't be disabled twice?
>
> What makes you think BH can't be disabled twice ?
>
> Look, I prefer being cautious here, no need to get another DCCP bug
> report later.

Hmm, I thought BH should have a local_bh_save() to save its context
but looking into its implementation it uses the preempt count to determine
if BH is disabled or not, unlikely hardirq's. Sorry for the noise.

Re: [PATCH net] dccp: fix use-after-free in dccp_feat_activate_values

[Eric Dumazet]

On Sun, 2017-03-05 at 21:38 -0800, Cong Wang wrote:

> Do you really want to disable BH again here?
> 
> dccp_check_req() should be always called on RX path where BH
> is already disabled and BH can't be disabled twice?

What makes you think BH can't be disabled twice ?

Look, I prefer being cautious here, no need to get another DCCP bug
report later.

Re: [PATCH net] dccp: fix use-after-free in dccp_feat_activate_values

[Cong Wang]

On Sun, Mar 5, 2017 at 10:52 AM, Eric Dumazet  wrote:
> --- a/net/dccp/minisocks.c
> +++ b/net/dccp/minisocks.c
> @@ -142,6 +142,13 @@ struct sock *dccp_check_req(struct sock *sk, struct 
> sk_buff *skb,
> struct dccp_request_sock *dreq = dccp_rsk(req);
> bool own_req;
>
> +   /* TCP/DCCP listeners became lockless.
> +* DCCP stores complex state in its request_sock, so we need
> +* a protection for them, now this code runs without being protected
> +* by the parent (listener) lock.
> +*/
> +   spin_lock_bh(>dreq_lock);
> +

Do you really want to disable BH again here?

dccp_check_req() should be always called on RX path where BH
is already disabled and BH can't be disabled twice?

[PATCH net] dccp: fix use-after-free in dccp_feat_activate_values

[Eric Dumazet]

From: Eric Dumazet 

Dmitry reported crashes in DCCP stack [1]

Problem here is that when I got rid of listener spinlock, I missed the
fact that DCCP stores a complex state in struct dccp_request_sock,
while TCP does not.

Since multiple cpus could access it at the same time, we need to add
protection.

[1]
BUG: KASAN: use-after-free in dccp_feat_activate_values+0x967/0xab0
net/dccp/feat.c:1541 at addr 88003713be68
Read of size 8 by task syz-executor2/8457
CPU: 2 PID: 8457 Comm: syz-executor2 Not tainted 4.10.0-rc7+ #127
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 
 __dump_stack lib/dump_stack.c:15 [inline]
 dump_stack+0x292/0x398 lib/dump_stack.c:51
 kasan_object_err+0x1c/0x70 mm/kasan/report.c:162
 print_address_description mm/kasan/report.c:200 [inline]
 kasan_report_error mm/kasan/report.c:289 [inline]
 kasan_report.part.1+0x20e/0x4e0 mm/kasan/report.c:311
 kasan_report mm/kasan/report.c:332 [inline]
 __asan_report_load8_noabort+0x29/0x30 mm/kasan/report.c:332
 dccp_feat_activate_values+0x967/0xab0 net/dccp/feat.c:1541
 dccp_create_openreq_child+0x464/0x610 net/dccp/minisocks.c:121
 dccp_v6_request_recv_sock+0x1f6/0x1960 net/dccp/ipv6.c:457
 dccp_check_req+0x335/0x5a0 net/dccp/minisocks.c:186
 dccp_v6_rcv+0x69e/0x1d00 net/dccp/ipv6.c:711
 ip6_input_finish+0x46d/0x17a0 net/ipv6/ip6_input.c:279
 NF_HOOK include/linux/netfilter.h:257 [inline]
 ip6_input+0xdb/0x590 net/ipv6/ip6_input.c:322
 dst_input include/net/dst.h:507 [inline]
 ip6_rcv_finish+0x289/0x890 net/ipv6/ip6_input.c:69
 NF_HOOK include/linux/netfilter.h:257 [inline]
 ipv6_rcv+0x12ec/0x23d0 net/ipv6/ip6_input.c:203
 __netif_receive_skb_core+0x1ae5/0x3400 net/core/dev.c:4190
 __netif_receive_skb+0x2a/0x170 net/core/dev.c:4228
 process_backlog+0xe5/0x6c0 net/core/dev.c:4839
 napi_poll net/core/dev.c:5202 [inline]
 net_rx_action+0xe70/0x1900 net/core/dev.c:5267
 __do_softirq+0x2fb/0xb7d kernel/softirq.c:284
 do_softirq_own_stack+0x1c/0x30 arch/x86/entry/entry_64.S:902
 
 do_softirq.part.17+0x1e8/0x230 kernel/softirq.c:328
 do_softirq kernel/softirq.c:176 [inline]
 __local_bh_enable_ip+0x1f2/0x200 kernel/softirq.c:181
 local_bh_enable include/linux/bottom_half.h:31 [inline]
 rcu_read_unlock_bh include/linux/rcupdate.h:971 [inline]
 ip6_finish_output2+0xbb0/0x23d0 net/ipv6/ip6_output.c:123
 ip6_finish_output+0x302/0x960 net/ipv6/ip6_output.c:148
 NF_HOOK_COND include/linux/netfilter.h:246 [inline]
 ip6_output+0x1cb/0x8d0 net/ipv6/ip6_output.c:162
 ip6_xmit+0xcdf/0x20d0 include/net/dst.h:501
 inet6_csk_xmit+0x320/0x5f0 net/ipv6/inet6_connection_sock.c:179
 dccp_transmit_skb+0xb09/0x1120 net/dccp/output.c:141
 dccp_xmit_packet+0x215/0x760 net/dccp/output.c:280
 dccp_write_xmit+0x168/0x1d0 net/dccp/output.c:362
 dccp_sendmsg+0x79c/0xb10 net/dccp/proto.c:796
 inet_sendmsg+0x164/0x5b0 net/ipv4/af_inet.c:744
 sock_sendmsg_nosec net/socket.c:635 [inline]
 sock_sendmsg+0xca/0x110 net/socket.c:645
 SYSC_sendto+0x660/0x810 net/socket.c:1687
 SyS_sendto+0x40/0x50 net/socket.c:1655
 entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x4458b9
RSP: 002b:7f8ceb77bb58 EFLAGS: 0282 ORIG_RAX: 002c
RAX: ffda RBX: 0017 RCX: 004458b9
RDX: 0023 RSI: 20e6 RDI: 0017
RBP: 006e1b90 R08: 200f9fe1 R09: 0020
R10: 8010 R11: 0282 R12: 007080a8
R13:  R14: 7f8ceb77c9c0 R15: 7f8ceb77c700
Object at 88003713be50, in cache kmalloc-64 size: 64
Allocated:
PID = 8446
 save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
 save_stack+0x43/0xd0 mm/kasan/kasan.c:502
 set_track mm/kasan/kasan.c:514 [inline]
 kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:605
 kmem_cache_alloc_trace+0x82/0x270 mm/slub.c:2738
 kmalloc include/linux/slab.h:490 [inline]
 dccp_feat_entry_new+0x214/0x410 net/dccp/feat.c:467
 dccp_feat_push_change+0x38/0x220 net/dccp/feat.c:487
 __feat_register_sp+0x223/0x2f0 net/dccp/feat.c:741
 dccp_feat_propagate_ccid+0x22b/0x2b0 net/dccp/feat.c:949
 dccp_feat_server_ccid_dependencies+0x1b3/0x250 net/dccp/feat.c:1012
 dccp_make_response+0x1f1/0xc90 net/dccp/output.c:423
 dccp_v6_send_response+0x4ec/0xc20 net/dccp/ipv6.c:217
 dccp_v6_conn_request+0xaba/0x11b0 net/dccp/ipv6.c:377
 dccp_rcv_state_process+0x51e/0x1650 net/dccp/input.c:606
 dccp_v6_do_rcv+0x213/0x350 net/dccp/ipv6.c:632
 sk_backlog_rcv include/net/sock.h:893 [inline]
 __sk_receive_skb+0x36f/0xcc0 net/core/sock.c:479
 dccp_v6_rcv+0xba5/0x1d00 net/dccp/ipv6.c:742
 ip6_input_finish+0x46d/0x17a0 net/ipv6/ip6_input.c:279
 NF_HOOK include/linux/netfilter.h:257 [inline]
 ip6_input+0xdb/0x590 net/ipv6/ip6_input.c:322
 dst_input include/net/dst.h:507 [inline]
 ip6_rcv_finish+0x289/0x890 net/ipv6/ip6_input.c:69
 NF_HOOK include/linux/netfilter.h:257 [inline]
 ipv6_rcv+0x12ec/0x23d0 net/ipv6/ip6_input.c:203
 __netif_receive_skb_core+0x1ae5/0x3400 net/core/dev.c:4190