Re: [PATCH nf 1/1] netfilter: nf_dup: prevent asynchronous duplicate recursion

[Florian Westphal]

Zihan Xi <zihanx@xxxxxxxxxx> wrote:
> nf_dup_ipv4() and nf_dup_ipv6() use current->in_nf_duplicate to keep
> duplicated packets from being duplicated again while ip_local_out() or
> ip6_local_out() walks netfilter hooks. The task flag is cleared as soon
> as the output function returns.
> 
> NFQUEUE can retain a duplicate and return from the output hook. A later
> NF_ACCEPT verdict resumes the same skb at the following hook from the
> verdict task, after in_nf_duplicate has been cleared. A later TEE target
> or dup expression can then duplicate it again. With an earlier queue
> hook and a later duplication hook, one packet can sustain an unbounded
> packet generation loop.
> 
> Record the duplication state in the cloned skb as well as the task. The
> skb flag survives queuing, reinjection, and skb metadata copies, so an
> asynchronously resumed duplicate cannot enter either IPv4 or IPv6
> duplication helper again. Copy the flag through nf_copy() so fragments
> retain the same state. Keep the task flag for the nested xtables jumpstack.

Not sure about this one.  I think this is a case of "behaves as
intended", you get oops because you *ask* the kernel to oops.

I think there are a great many other ways to OOM the kernel,
outside of dup/TEE/nfqueue.

IFF we prented that this is a real problem, then I would
prefer to solve this in nf_dup, not involving sk_buff changes.

I cannot see a sensible use case for nf_dup outside of
physical hardware (sending packets to some external
packet logging machine for instance).

So. I think a better solution would be to either disable
dup in user namespaces entirely, or, restore the "old" behaviour
of passing the clone directly (no reentry) if we were configured
from user namespace.