On Tue, Oct 22, 2019 at 06:56:42PM +0200, Florian Westphal wrote:
> syzbot reported following splat:
> BUG: KASAN: use-after-free in __nf_ct_ext_exist
> include/net/netfilter/nf_conntrack_extend.h:53 [inline]
> BUG: KASAN: use-after-free in nf_ct_deliver_cached_events+0x5c3/0x6d0
> net/netfilter/nf_conntrack_ecache.c:205
> nf_conntrack_confirm include/net/netfilter/nf_conntrack_core.h:65 [inline]
> nf_confirm+0x3d8/0x4d0 net/netfilter/nf_conntrack_proto.c:154
> [..]
>
> While there is no reproducer yet, the syzbot report contains one
> interesting bit of information:
>
> Freed by task 27585:
> [..]
> kfree+0x10a/0x2c0 mm/slab.c:3757
> nf_ct_ext_destroy+0x2ab/0x2e0 net/netfilter/nf_conntrack_extend.c:38
> nf_conntrack_free+0x8f/0xe0 net/netfilter/nf_conntrack_core.c:1418
> destroy_conntrack+0x1a2/0x270 net/netfilter/nf_conntrack_core.c:626
> nf_conntrack_put include/linux/netfilter/nf_conntrack_common.h:31 [inline]
> nf_ct_resolve_clash net/netfilter/nf_conntrack_core.c:915 [inline]
> ^^^^^^^^^^^^^^^^^^^
> __nf_conntrack_confirm+0x21ca/0x2830 net/netfilter/nf_conntrack_core.c:1038
> nf_conntrack_confirm include/net/netfilter/nf_conntrack_core.h:63 [inline]
> nf_confirm+0x3e7/0x4d0 net/netfilter/nf_conntrack_proto.c:154
>
> This is whats happening:
>
> 1. a conntrack entry is about to be confirmed (added to hash table).
> 2. a clash with existing entry is detected.
> 3. nf_ct_resolve_clash() puts skb->nfct (the "losing" entry).
> 4. this entry now has a refcount of 0 and is freed to SLAB_TYPESAFE_BY_RCU
> kmem cache.
>
> skb->nfct has been replaced by the one found in the hash.
> Problem is that nf_conntrack_confirm() uses the old ct:
>
> static inline int nf_conntrack_confirm(struct sk_buff *skb)
> {
> struct nf_conn *ct = (struct nf_conn *)skb_nfct(skb);
> int ret = NF_ACCEPT;
>
> if (ct) {
> if (!nf_ct_is_confirmed(ct))
> ret = __nf_conntrack_confirm(skb);
> if (likely(ret == NF_ACCEPT))
> nf_ct_deliver_cached_events(ct); /* This ct has refcount 0! */
> }
> return ret;
> }
>
> As of "netfilter: conntrack: free extension area immediately", we can't
> access conntrack extensions in this case.
>
> To fix this, make sure we check the dying bit presence before attempting
> to get the eache extension.
Applied, thanks.
> Reported-by: syzbot+c7aabc9fe93e7f3637ba@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: 2ad9d7747c10d1 ("netfilter: conntrack: free extension area immediately")
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> ---
> I plan to send a patch for nf tree to alter nf_conntrack_confirm()
> to not cache the ct -- I think its a bug too, we should call
> nf_ct_deliver_cached_events() on the ct that is assigned to skb *now*,
> not the old one.
This is the clash resolution that is triggering this path you describe
in this note.
