Re: [syzbot] [netfilter?] KASAN: slab-use-after-free Read in nft_array_get_cmp

[Florian Westphal <fw@strlen.de>]

Florian Westphal <fw@strlen.de> wrote:
> >  dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
> >  print_address_description mm/kasan/report.c:378 [inline]
> >  print_report+0xba/0x230 mm/kasan/report.c:482
> >  kasan_report+0x117/0x150 mm/kasan/report.c:595
> >  nft_set_ext include/net/netfilter/nf_tables.h:795 [inline]
> >  nft_set_ext_key include/net/netfilter/nf_tables.h:800 [inline]
> >  nft_array_get_cmp+0x1f6/0x2a0 net/netfilter/nft_set_rbtree.c:133
> >  __inline_bsearch include/linux/bsearch.h:15 [inline]
> >  bsearch+0x50/0xc0 lib/bsearch.c:33
> >  nft_rbtree_get+0x16b/0x400 net/netfilter/nft_set_rbtree.c:169
> 
> Use after free, its possible for rbtree insert to evict elements
> due to expiry, but still return -EEXIST.
> 
> We can thus end up with removed elements but without a call to
> ->commit(), hence we have a stale binary blob referencing free'd
> elements.

So far I found 3 related issues, bsearch blob gets out of sync with the
rbtree.  Prerequisite: set has a timed-out entry.  In all cases
the bsearch blob isn't refreshed and holds pointers to free'd elements.

1. add new element, transaction is later aborted.
Fix: refresh the bsearch blob from abort callback.

2. re-add an existing element, transaction passes.
In this case, the commit hook isn't called because we don't have
any changes to the set from transaction point of view.

3. create (F_EXCL) an existing element.  This also
aborts the transaction, but the abort callback isn't run
as no change to the set was made.

I'm not yet sure how to fix 2 and 3.

I'll see about moving set_update_list to the nft pernet
structure, so it can be populated from
nft_trans_gc_queue_sync_done().

Pipapo set backend should be fine with it, it won't act
when called with the 'private clone' being NULL.