Re: [PATCH nf] netfilter: nf_ct_ext: fix possible panic after nf_ct_extend_unregister

[Florian Westphal <fw@strlen.de>]

Liping Zhang <zlpnobody@163.com> wrote:
> Step 1. Enable SYNPROXY for tcp dport 1234 at FORWARD hook:
>   # iptables -I FORWARD -p tcp --dport 1234 -j SYNPROXY
> Step 2. Queue the syn packet to the userspace at raw table OUTPUT hook.
>         Also note, in the userspace we only add a 20s' delay, then
>         reinject the syn packet to the kernel:
>   # iptables -t raw -I OUTPUT -p tcp --syn -j NFQUEUE --queue-num 1
> Step 3. Using "nc 2.2.2.2 1234" to connect the server.
> Step 4. Now remove the nf_synproxy_core.ko quickly:
>   # iptables -F FORWARD
>   # rmmod ipt_SYNPROXY
>   # rmmod nf_synproxy_core
> Step 5. After 20s' delay, the syn packet is reinjected to the kernel.

Lovely.

> But having such a obscure restriction of nf_ct_extend_unregister is not a
> good idea, so we should invoke synchronize_rcu after set nf_ct_ext_types
> to NULL, and check the NULL pointer when do __nf_ct_ext_add_length. Then
> it will be easier if we add new ct extend in the future.

Agree.

Acked-by: Florian Westphal <fw@strlen.de>

> Last, we use kfree_rcu to free nf_ct_ext, so rcu_barrier() is unnecessary
> anymore, remove it too.

I think with some extra work we could switch to kfree since almost
all spots that access the extension area do it after obtaining
a reference on the conntrack.

Someone would need to audit the code first, I suspect the ecache work
queue isn't safe without the kfree_rcu, perhaps there are other places
as well.