From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [PATCH net] net/sched: act_sample: fix NULL dereference in the data path Date: Fri, 14 Sep 2018 12:02:47 +0200 Message-ID: <20180914100247.GN25110@nanopsycho> References: <2fcef6665503c5e3b17676daf45c4166cf130cdb.1536919144.git.dcaratti@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: mcroce@redhat.com, Jamal Hadi Salim , Cong Wang , "David S. Miller" , netdev@vger.kernel.org To: Davide Caratti Return-path: Received: from mail-wr1-f67.google.com ([209.85.221.67]:33930 "EHLO mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728007AbeINPUs (ORCPT ); Fri, 14 Sep 2018 11:20:48 -0400 Received: by mail-wr1-f67.google.com with SMTP id g33-v6so9931705wrd.1 for ; Fri, 14 Sep 2018 03:07:00 -0700 (PDT) Content-Disposition: inline In-Reply-To: <2fcef6665503c5e3b17676daf45c4166cf130cdb.1536919144.git.dcaratti@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: Fri, Sep 14, 2018 at 12:03:18PM CEST, dcaratti@redhat.com wrote: >Matteo reported the following splat, testing the datapath of TC 'sample': > > BUG: KASAN: null-ptr-deref in tcf_sample_act+0xc4/0x310 > Read of size 8 at addr 0000000000000000 by task nc/433 > > CPU: 0 PID: 433 Comm: nc Not tainted 4.19.0-rc3-kvm #17 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS ?-20180531_142017-buildhw-08.phx2.fedoraproject.org-1.fc28 04/01/2014 > Call Trace: > kasan_report.cold.6+0x6c/0x2fa > tcf_sample_act+0xc4/0x310 > ? dev_hard_start_xmit+0x117/0x180 > tcf_action_exec+0xa3/0x160 > tcf_classify+0xdd/0x1d0 > htb_enqueue+0x18e/0x6b0 > ? deref_stack_reg+0x7a/0xb0 > ? htb_delete+0x4b0/0x4b0 > ? unwind_next_frame+0x819/0x8f0 > ? entry_SYSCALL_64_after_hwframe+0x44/0xa9 > __dev_queue_xmit+0x722/0xca0 > ? unwind_get_return_address_ptr+0x50/0x50 > ? netdev_pick_tx+0xe0/0xe0 > ? save_stack+0x8c/0xb0 > ? kasan_kmalloc+0xbe/0xd0 > ? __kmalloc_track_caller+0xe4/0x1c0 > ? __kmalloc_reserve.isra.45+0x24/0x70 > ? __alloc_skb+0xdd/0x2e0 > ? sk_stream_alloc_skb+0x91/0x3b0 > ? tcp_sendmsg_locked+0x71b/0x15a0 > ? tcp_sendmsg+0x22/0x40 > ? __sys_sendto+0x1b0/0x250 > ? __x64_sys_sendto+0x6f/0x80 > ? do_syscall_64+0x5d/0x150 > ? entry_SYSCALL_64_after_hwframe+0x44/0xa9 > ? __sys_sendto+0x1b0/0x250 > ? __x64_sys_sendto+0x6f/0x80 > ? do_syscall_64+0x5d/0x150 > ? entry_SYSCALL_64_after_hwframe+0x44/0xa9 > ip_finish_output2+0x495/0x590 > ? ip_copy_metadata+0x2e0/0x2e0 > ? skb_gso_validate_network_len+0x6f/0x110 > ? ip_finish_output+0x174/0x280 > __tcp_transmit_skb+0xb17/0x12b0 > ? __tcp_select_window+0x380/0x380 > tcp_write_xmit+0x913/0x1de0 > ? __sk_mem_schedule+0x50/0x80 > tcp_sendmsg_locked+0x49d/0x15a0 > ? tcp_rcv_established+0x8da/0xa30 > ? tcp_set_state+0x220/0x220 > ? clear_user+0x1f/0x50 > ? iov_iter_zero+0x1ae/0x590 > ? __fget_light+0xa0/0xe0 > tcp_sendmsg+0x22/0x40 > __sys_sendto+0x1b0/0x250 > ? __ia32_sys_getpeername+0x40/0x40 > ? _copy_to_user+0x58/0x70 > ? poll_select_copy_remaining+0x176/0x200 > ? __pollwait+0x1c0/0x1c0 > ? ktime_get_ts64+0x11f/0x140 > ? kern_select+0x108/0x150 > ? core_sys_select+0x360/0x360 > ? vfs_read+0x127/0x150 > ? kernel_write+0x90/0x90 > __x64_sys_sendto+0x6f/0x80 > do_syscall_64+0x5d/0x150 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > RIP: 0033:0x7fefef2b129d > Code: ff ff ff ff eb b6 0f 1f 80 00 00 00 00 48 8d 05 51 37 0c 00 41 89 ca 8b 00 85 c0 75 20 45 31 c9 45 31 c0 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 6b f3 c3 66 0f 1f 84 00 00 00 00 00 41 56 41 > RSP: 002b:00007fff2f5350c8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c > RAX: ffffffffffffffda RBX: 000056118d60c120 RCX: 00007fefef2b129d > RDX: 0000000000002000 RSI: 000056118d629320 RDI: 0000000000000003 > RBP: 000056118d530370 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000002000 > R13: 000056118d5c2a10 R14: 000056118d5c2a10 R15: 000056118d5303b8 > >tcf_sample_act() tried to update its per-cpu stats, but tcf_sample_init() >forgot to allocate them, because tcf_idr_create() was called with a wrong >value of 'cpustats'. Setting it to true proved to fix the reported crash. > >Reported-by: Matteo Croce >Fixes: 65a206c01e8e ("net/sched: Change act_api and act_xxx modules to use IDR") >Fixes: 5c5670fae430 ("net/sched: Introduce sample tc action") >Tested-by: Matteo Croce >Signed-off-by: Davide Caratti Acked-by: Jiri Pirko