Re: netfilter: nf_nat: race in nf_nat_setup_info() corrupts nat_bysource list (GPF / soft lockup)

Hi Florian,

I do see some conntrack updates passed nf_nat_initialized check in
nf_nat_setup_info so ideally it should not do anything for the second
or other run for the same conntrack.
How about locking out the whole of nf_nat_setup_info on ct->lock. i..e
spin_lock_bh(&ct->lock) at the start of the function and unlock before
return.

On Sun, Aug 16, 2026 at 7:55 PM Florian Westphal <[email protected]> wrote:
>
> Vimal Agrawal <[email protected]> wrote:
> > Our custom module can decide to send the first packet of a conntrack
> > to userspace via NFQUEUE while the rest of the packets for that
> > conntrack follow the standard kernel forward path.
> > In the case of a bridge, br_flood() clones skbs, and those clones
> > share one unconfirmed conntrack for the initial packet(s) (via
> > skb_clone()'s __nf_copy(), which just bumps the refcount and copies
> > the same ct pointer).
> >
> > br_flood() clones the skb; Skb1 and Skb2 share one unconfirmed ct:
> >   Skb1 (cpu1): -> nfqueue -> [userspace verdict] -> reinject -> SNAT
> > -> confirm (cpu2)
> >   Skb2 (cpu1): -> SNAT -> confirm (cpu1)
> > Skb1's reinject-triggered SNAT (cpu2) races Skb2's synchronous SNAT
> > (cpu1) on the same, still-unconfirmed ct.
> >
> > Because of this, SNAT for the two skbs sharing the unconfirmed
> > conntrack races across two different CPUs. nf_nat_setup_info() sees on
> > both CPUs that the conntrack isn't yet SNAT-initialized, and both try
> > to initialize it — pushing the same conntrack node onto the
> > nat_bysource hash chain twice, corrupting the list.
>
> Yes, conntrack assumes exclusive access, this is why nfqueue has all
> these hacks to drop cloned skbs where we don't own the unconfirmed skb,
> e.g. in nfqnl_enqueue_packet().
>
> We could add more hacks but "fixing" nf_nat_setup_info() doesn't really
> help, its only one of many possible races.
>
> If you want to add more hacks, you could move:
>
>         /* It's done. */
>         if (maniptype == NF_NAT_MANIP_DST)
>                 ct->status |= IPS_DST_NAT_DONE;
>         else
>                 ct->status |= IPS_SRC_NAT_DONE;
>
> in nf_nat_setup_info to test_and_Set_bit() done while holding the
> lock to add to unconfirmed list, failure causes drop.
> But it won't solve the underlying design defects.