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

On Sat, Aug 29, 2026 at 5:27 PM Florian Westphal <[email protected]> wrote:
>
> Zihan Xi <[email protected]> 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.

Hi Florian,

Thanks for the review and for outlining the two possible directions.

I take your suggestion to mean that, if we decide this is worth addressing, the
fix should stay within nf_dup rather than add state to struct sk_buff. Before
deciding whether to pursue a patch, I rechecked the practical impact.

The current reproducer intentionally drives the system into OOM, and this is
not a memory-corruption issue. An unprivileged trigger requires user and
network namespaces, NFQUEUE, TEE/dup support, and a userspace verdict service.
CAP_NET_ADMIN is available in the child network namespace when user namespaces
are enabled. However, vm.panic_on_oom is global and cannot be enabled by the
child namespace. With the normal panic_on_oom=0 setting, the demonstrated effect
is resource pressure and possible OOM-killer activity, rather than a direct
kernel panic. I have not demonstrated privilege escalation or a memory-safety
impact.

The crash log in the cover letter came from a separate root validation run with
panic_on_oom=1. It should not be taken as evidence that an unprivileged user can
directly panic the host.

Based on this reassessment, I do not think the original struct sk_buff guard
series is justified. My current preference is therefore to withdraw that
series, unless you think a defense-in-depth change is still desirable.

If you do prefer to keep a small nf_dup-only change, I can prepare either:

1. disable IPv4/IPv6 dup in network namespaces owned by a non-initial user
   namespace; or
2. restore the historical direct ip_output()/ip6_output() path there.

The second option preserves one duplicate but bypasses the clone's LOCAL_OUT and
POST_ROUTING netfilter processing, so it changes observable semantics. The
first option removes dup/TEE functionality in those namespaces but avoids that
hook bypass.

Please let me know whether you prefer that I withdraw the series or prepare one
of these defense-in-depth changes. I will hold off on sending a v2 of the
struct sk_buff-field patch until then.

Best regards,
Zihan Xi