syszbot found an interesting use-after-free [1] happening while IPv4 fragment rhashtable was destroyed at netns dismantle.
While no insertions can possibly happen at the time a dismantling netns is destroying this rhashtable, timers can still fire and attempt to remove elements from this rhashtable. This is forbidden, since rhashtable_free_and_destroy() has no synchronization against concurrent inserts and deletes. Add a new nf->dead flag so that timers do not attempt a rhashtable_remove_fast() operation. [1] BUG: KASAN: use-after-free in __read_once_size include/linux/compiler.h:188 [inline] BUG: KASAN: use-after-free in rhashtable_last_table+0x216/0x240 lib/rhashtable.c:217 Read of size 8 at addr ffff88019a4c8840 by task kworker/0:4/8279 CPU: 0 PID: 8279 Comm: kworker/0:4 Not tainted 4.19.0-rc5+ #61 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: events rht_deferred_worker Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433 __read_once_size include/linux/compiler.h:188 [inline] rhashtable_last_table+0x216/0x240 lib/rhashtable.c:217 rht_deferred_worker+0x157/0x1de0 lib/rhashtable.c:410 process_one_work+0xc90/0x1b90 kernel/workqueue.c:2153 worker_thread+0x17f/0x1390 kernel/workqueue.c:2296 kthread+0x35a/0x420 kernel/kthread.c:246 ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413 Allocated by task 5: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553 __do_kmalloc_node mm/slab.c:3682 [inline] __kmalloc_node+0x47/0x70 mm/slab.c:3689 kmalloc_node include/linux/slab.h:555 [inline] kvmalloc_node+0xb9/0xf0 mm/util.c:423 kvmalloc include/linux/mm.h:577 [inline] kvzalloc include/linux/mm.h:585 [inline] bucket_table_alloc+0x9a/0x4e0 lib/rhashtable.c:176 rhashtable_rehash_alloc+0x73/0x100 lib/rhashtable.c:353 rht_deferred_worker+0x278/0x1de0 lib/rhashtable.c:413 process_one_work+0xc90/0x1b90 kernel/workqueue.c:2153 worker_thread+0x17f/0x1390 kernel/workqueue.c:2296 kthread+0x35a/0x420 kernel/kthread.c:246 ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413 Freed by task 8283: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521 kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528 __cache_free mm/slab.c:3498 [inline] kfree+0xcf/0x230 mm/slab.c:3813 kvfree+0x61/0x70 mm/util.c:452 bucket_table_free+0xda/0x250 lib/rhashtable.c:108 rhashtable_free_and_destroy+0x152/0x900 lib/rhashtable.c:1163 inet_frags_exit_net+0x3d/0x50 net/ipv4/inet_fragment.c:96 ipv4_frags_exit_net+0x73/0x90 net/ipv4/ip_fragment.c:914 ops_exit_list.isra.7+0xb0/0x160 net/core/net_namespace.c:153 cleanup_net+0x555/0xb10 net/core/net_namespace.c:551 process_one_work+0xc90/0x1b90 kernel/workqueue.c:2153 worker_thread+0x17f/0x1390 kernel/workqueue.c:2296 kthread+0x35a/0x420 kernel/kthread.c:246 ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413 The buggy address belongs to the object at ffff88019a4c8800 which belongs to the cache kmalloc-16384 of size 16384 The buggy address is located 64 bytes inside of 16384-byte region [ffff88019a4c8800, ffff88019a4cc800) The buggy address belongs to the page: page:ffffea0006693200 count:1 mapcount:0 mapping:ffff8801da802200 index:0x0 compound_mapcount: 0 flags: 0x2fffc0000008100(slab|head) raw: 02fffc0000008100 ffffea0006685608 ffffea0006617c08 ffff8801da802200 raw: 0000000000000000 ffff88019a4c8800 0000000100000001 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff88019a4c8700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff88019a4c8780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc >ffff88019a4c8800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ^ ffff88019a4c8880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff88019a4c8900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb Fixes: 648700f76b03 ("inet: frags: use rhashtables for reassembly units") Signed-off-by: Eric Dumazet <eduma...@google.com> Reported-by: syzbot <syzkal...@googlegroups.com> Cc: Thomas Graf <tg...@suug.ch> Cc: Herbert Xu <herb...@gondor.apana.org.au> --- include/net/inet_frag.h | 4 +++- net/ipv4/inet_fragment.c | 31 +++++++++++++++++++++---------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h index 1662cbc0b46b45296a367ecbdaf03c68854fdce7..ffe5e1be40212fa63e360f3e29a56c1b2ce897ee 100644 --- a/include/net/inet_frag.h +++ b/include/net/inet_frag.h @@ -11,7 +11,7 @@ struct netns_frags { int timeout; int max_dist; struct inet_frags *f; - + bool dead; struct rhashtable rhashtable ____cacheline_aligned_in_smp; /* Keep atomic mem on separate cachelines in structs that include it */ @@ -24,11 +24,13 @@ struct netns_frags { * @INET_FRAG_FIRST_IN: first fragment has arrived * @INET_FRAG_LAST_IN: final fragment has arrived * @INET_FRAG_COMPLETE: frag queue has been processed and is due for destruction + * @INET_FRAG_HASH_DEAD: inet_frag_kill() has not removed fq from rhashtable */ enum { INET_FRAG_FIRST_IN = BIT(0), INET_FRAG_LAST_IN = BIT(1), INET_FRAG_COMPLETE = BIT(2), + INET_FRAG_HASH_DEAD = BIT(3), }; struct frag_v4_compare_key { diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c index bcb11f3a27c0c34115af05034a5a20f57842eb0a..887134425350cc7c8ee081d8c83a037074d96d93 100644 --- a/net/ipv4/inet_fragment.c +++ b/net/ipv4/inet_fragment.c @@ -71,28 +71,31 @@ EXPORT_SYMBOL(inet_frags_fini); static void inet_frags_free_cb(void *ptr, void *arg) { struct inet_frag_queue *fq = ptr; + int count = 0; - /* If we can not cancel the timer, it means this frag_queue - * is already disappearing, we have nothing to do. - * Otherwise, we own a refcount until the end of this function. - */ - if (!del_timer(&fq->timer)) - return; + if (del_timer_sync(&fq->timer)) + count++; spin_lock_bh(&fq->lock); if (!(fq->flags & INET_FRAG_COMPLETE)) { fq->flags |= INET_FRAG_COMPLETE; - refcount_dec(&fq->refcnt); + count++; + } else if (fq->flags & INET_FRAG_HASH_DEAD) { + count++; } spin_unlock_bh(&fq->lock); - inet_frag_put(fq); + if (refcount_sub_and_test(count, &fq->refcnt)) + inet_frag_destroy(fq); } void inet_frags_exit_net(struct netns_frags *nf) { nf->high_thresh = 0; /* prevent creation of new frags */ + /* paired with READ_ONCE() in inet_frag_kill() */ + smp_store_release(&nf->dead, true); + rhashtable_free_and_destroy(&nf->rhashtable, inet_frags_free_cb, NULL); } EXPORT_SYMBOL(inet_frags_exit_net); @@ -106,8 +109,16 @@ void inet_frag_kill(struct inet_frag_queue *fq) struct netns_frags *nf = fq->net; fq->flags |= INET_FRAG_COMPLETE; - rhashtable_remove_fast(&nf->rhashtable, &fq->node, nf->f->rhash_params); - refcount_dec(&fq->refcnt); + /* This READ_ONCE() is paired with smp_store_release() + * in inet_frags_exit_net(). + */ + if (!READ_ONCE(nf->dead)) { + rhashtable_remove_fast(&nf->rhashtable, &fq->node, + nf->f->rhash_params); + refcount_dec(&fq->refcnt); + } else { + fq->flags |= INET_FRAG_HASH_DEAD; + } } } EXPORT_SYMBOL(inet_frag_kill); -- 2.19.0.605.g01d371f741-goog