Subject: Re: [Patch net-next v3] net_sched: change tcf_del_walker() to take idrinfo->lock

From: Ido Schimmel <[email-masked]>
Date: Fri, 28 Sep 2018 17:59:00 +0300

On Wed, Sep 19, 2018 at 04:37:29PM -0700, Cong Wang wrote:
> From: Vlad Buslov <[email-masked]>
> 
> From: Vlad Buslov <[email-masked]>
> 
> Action API was changed to work with actions and action_idr in concurrency
> safe manner, however tcf_del_walker() still uses actions without taking a
> reference or idrinfo->lock first, and deletes them directly, disregarding
> possible concurrent delete.
> 
> Change tcf_del_walker() to take idrinfo->lock while iterating over actions
> and use new tcf_idr_release_unsafe() to release them while holding the
> lock.
> 
> And the blocking function fl_hw_destroy_tmplt() could be called when we
> put a filter chain, so defer it to a work queue.

I'm getting a use-after-free when running tc_chains.sh selftest and I
believe it's caused by this patch.

To reproduce:
# cd tools/testing/selftests/net/forwarding
# export TESTS="template_filter_fits"; ./tc_chains.sh veth0 veth1

__tcf_chain_put()
	tc_chain_tmplt_del()
		fl_tmplt_destroy()
			tcf_queue_work(&tmplt->rwork, fl_tmplt_destroy_work)
	tcf_chain_destroy()
		kfree(chain)

Some time later fl_tmplt_destroy_work() starts executing and
dereferencing 'chain'.

Splat:
[   48.269074] ==================================================================
[   48.270186] BUG: KASAN: use-after-free in fl_tmplt_destroy_work+0x289/0x2d0
[   48.271199] Read of size 8 at addr ffff880067f0d498 by task kworker/u2:1/18
[   48.272270]                                        
[   48.272520] CPU: 0 PID: 18 Comm: kworker/u2:1 Not tainted 4.19.0-rc5-custom+ #917      
[   48.273683] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20180531_142017-buildhw-08.phx2.fedoraproject.org-1.fc28 04/01/2014
[   48.275495] Workqueue: tc_filter_workqueue fl_tmplt_destroy_work                    
[   48.276387] Call Trace:                                                             
[   48.276766]  dump_stack+0x10f/0x1d8                        
[   48.277302]  ? dump_stack_print_info.cold.0+0x20/0x20
[   48.278080]  ? printk+0xac/0xd4                   
[   48.278585]  ? cpumask_weight.constprop.23+0x39/0x39                          
[   48.279360]  print_address_description.cold.3+0x9/0x258
[   48.280186]  kasan_report.cold.4+0x6a/0x9d
[   48.280856]  __asan_report_load8_noabort+0x19/0x20
[   48.281642]  fl_tmplt_destroy_work+0x289/0x2d0                                
[   48.282331]  ? fl_tmplt_destroy+0x30/0x30                                     
[   48.282966]  process_one_work+0xb5d/0x1a10                                    
[   48.283642]  ? kasan_check_write+0x14/0x20                  
[   48.284301]  ? pwq_dec_nr_in_flight+0x4e0/0x4e0                        
[   48.285003]  ? lock_repin_lock+0x360/0x360                                                                               
[   48.285652]  ? __schedule+0x86d/0x2310
[   48.286256]  ? lockdep_hardirqs_on+0x39f/0x580
[   48.286995]  ? __sched_text_start+0x8/0x8
[   48.287654]  ? lock_downgrade+0x740/0x740
[   48.288292]  ? sched_clock_local+0xe0/0x150
[   48.288969]  ? save_trace+0x340/0x340
[   48.289526]  ? save_trace+0x340/0x340
[   48.290156]  ? check_preemption_disabled+0x3b/0x210
[   48.290937]  ? find_held_lock+0x40/0x1d0
[   48.291598]  ? debug_smp_processor_id+0x1c/0x20
[   48.292290]  ? worker_thread+0x3c3/0x12b0
[   48.292952]  ? lock_contended+0x1260/0x1260
[   48.293606]  ? _raw_spin_unlock_irq+0x2c/0x50
[   48.294311]  ? do_raw_spin_trylock+0x121/0x1c0
[   48.294982]  ? do_raw_spin_lock+0x1e0/0x1e0
[   48.295658]  ? trace_hardirqs_on+0x290/0x290
[   48.296346]  worker_thread+0x18e/0x12b0
[   48.296972]  ? lock_repin_lock+0x360/0x360
[   48.297624]  ? process_one_work+0x1a10/0x1a10
[   48.298332]  ? save_trace+0x340/0x340
[   48.298937]  ? kasan_check_write+0x14/0x20
[   48.299639]  ? sched_clock_local+0xe0/0x150
[   48.300310]  ? check_preemption_disabled+0x3b/0x210
[   48.301064]  ? check_preemption_disabled+0x3b/0x210
[   48.301848]  ? find_held_lock+0x40/0x1d0
[   48.302472]  ? debug_smp_processor_id+0x1c/0x20
[   48.303196]  ? _raw_spin_unlock_irqrestore+0x57/0x70
[   48.304037]  ? _raw_spin_unlock_irqrestore+0x57/0x70
[   48.304862]  ? lockdep_hardirqs_on+0x39f/0x580
[   48.305595]  ? trace_hardirqs_on+0xa1/0x290
[   48.306237]  ? do_raw_spin_trylock+0x121/0x1c0
[   48.306924]  ? __kthread_parkme+0xdc/0x1a0
[   48.307607]  ? preempt_schedule_common+0x1f/0xd0
[   48.308359]  ? __kthread_parkme+0x5d/0x1a0
[   48.309014]  ? __kthread_parkme+0xfa/0x1a0
[   48.309690]  ? _raw_spin_unlock_irqrestore+0x60/0x70
[   48.310461]  kthread+0x34d/0x410
[   48.310970]  ? process_one_work+0x1a10/0x1a10
[   48.311672]  ? kthread_delayed_work_timer_fn+0x450/0x450
[   48.312548]  ret_from_fork+0x24/0x30
[   48.313114]
[   48.313390] Allocated by task 1239:
[   48.313983]  save_stack+0x43/0xd0
[   48.314533]  kasan_kmalloc+0xc4/0xe0
[   48.315119]  kmem_cache_alloc_trace+0x124/0x280
[   48.315873]  tcf_chain_create+0xa4/0x370
[   48.316506]  tc_ctl_chain+0xe3d/0x1430
[   48.317114]  rtnetlink_rcv_msg+0x3a3/0xa80
[   48.317799]  netlink_rcv_skb+0x152/0x3c0
[   48.318440]  rtnetlink_rcv+0x21/0x30
[   48.319042]  netlink_unicast+0x52f/0x740
[   48.319675]  netlink_sendmsg+0x9c7/0xf50
[   48.320276]  sock_sendmsg+0xcf/0x120
[   48.320853]  ___sys_sendmsg+0x778/0x8f0
[   48.321445]  __sys_sendmsg+0x112/0x270
[   48.322079]  __x64_sys_sendmsg+0x7d/0xc0
[   48.322687]  do_syscall_64+0x15d/0x640
[   48.323274]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[   48.324004]
[   48.324264] Freed by task 1248:
[   48.324796]  save_stack+0x43/0xd0
[   48.314533]  kasan_kmalloc+0xc4/0xe0
[   48.315119]  kmem_cache_alloc_trace+0x124/0x280
[   48.315873]  tcf_chain_create+0xa4/0x370
[   48.316506]  tc_ctl_chain+0xe3d/0x1430
[   48.317114]  rtnetlink_rcv_msg+0x3a3/0xa80
[   48.317799]  netlink_rcv_skb+0x152/0x3c0
[   48.318440]  rtnetlink_rcv+0x21/0x30
[   48.319042]  netlink_unicast+0x52f/0x740
[   48.319675]  netlink_sendmsg+0x9c7/0xf50
[   48.320276]  sock_sendmsg+0xcf/0x120
[   48.320853]  ___sys_sendmsg+0x778/0x8f0
[   48.321445]  __sys_sendmsg+0x112/0x270
[   48.322079]  __x64_sys_sendmsg+0x7d/0xc0
[   48.322687]  do_syscall_64+0x15d/0x640
[   48.323274]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[   48.324004]
[   48.324264] Freed by task 1248:
[   48.324796]  save_stack+0x43/0xd0
[   48.325355]  __kasan_slab_free+0x131/0x180
[   48.326001]  kasan_slab_free+0xe/0x10
[   48.326579]  kfree+0xd7/0x2b0
[   48.327054]  __tcf_chain_put+0x339/0x710
[   48.327654]  tc_ctl_chain+0xbde/0x1430
[   48.328276]  rtnetlink_rcv_msg+0x3a3/0xa80
[   48.328899]  netlink_rcv_skb+0x152/0x3c0
[   48.329556]  rtnetlink_rcv+0x21/0x30
[   48.330170]  netlink_unicast+0x52f/0x740
[   48.330839]  netlink_sendmsg+0x9c7/0xf50
[   48.331491]  sock_sendmsg+0xcf/0x120
[   48.332079]  ___sys_sendmsg+0x778/0x8f0
[   48.332706]  __sys_sendmsg+0x112/0x270
[   48.333334]  __x64_sys_sendmsg+0x7d/0xc0
[   48.333957]  do_syscall_64+0x15d/0x640
[   48.334581]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[   48.335362]
[   48.335645] The buggy address belongs to the object at ffff880067f0d480
[   48.335645]  which belongs to the cache kmalloc-64 of size 64
[   48.337559] The buggy address is located 24 bytes inside of
[   48.337559]  64-byte region [ffff880067f0d480, ffff880067f0d4c0)
[   48.339313] The buggy address belongs to the page:
[   48.340083] page:ffffea00019fc340 count:1 mapcount:0 mapping:ffff88006cc01780 index:0x0
[   48.341330] flags: 0x100000000000100(slab)
[   48.342000] raw: 0100000000000100 ffffea00019fce80 0000001100000011 ffff88006cc01780
[   48.343163] raw: 0000000000000000 00000000002a002a 00000001ffffffff 0000000000000000
[   48.344424] page dumped because: kasan: bad access detected
[   48.345299]
[   48.345580] Memory state around the buggy address:
[   48.346356]  ffff880067f0d380: fb fb fb fb fc fc fc fc fb fb fb fb fb fb fb fb
[   48.347477]  ffff880067f0d400: fc fc fc fc fb fb fb fb fb fb fb fb fc fc fc fc
[   48.348645] >ffff880067f0d480: fb fb fb fb fb fb fb fb fc fc fc fc fb fb fb fb
[   48.349799]                             ^
[   48.350457]  ffff880067f0d500: fb fb fb fb fc fc fc fc fb fb fb fb fb fb fb fb
[   48.351627]  ffff880067f0d580: fc fc fc fc fb fb fb fb fb fb fb fb fc fc fc fc
[   48.352802] ==================================================================