Hi Florian,
On Fri, Aug 01, 2025 at 05:25:08PM +0200, Florian Westphal wrote:
> There is a reference count leak in ctnetlink_dump_table():
> if (res < 0) {
> nf_conntrack_get(&ct->ct_general); // HERE
> cb->args[1] = (unsigned long)ct;
> ...
goto out;
>
> While its very unlikely, its possible that ct == last.
out:
...
if (last) {
/* nf ct hash resize happened, now clear the leftover. */
if ((struct nf_conn *)cb->args[1] == last) {
cb->args[1] = 0;
}
nf_ct_put(last);
}
I think problem was introduced here:
fefa92679dbe ("netfilter: ctnetlink: fix incorrect nf_ct_put during hash resize")
> If this happens, then the refcount of ct was already incremented.
> This 2nd increment is never undone.
>
> This prevents the conntrack object from being released, which in turn
> keeps prevents cnet->count from dropping back to 0.
>
> This will then block the netns dismantle (or conntrack rmmod) as
> nf_conntrack_cleanup_net_list() will wait forever.
>
> This can be reproduced by running conntrack_resize.sh selftest in a loop.
> It takes ~20 minutes for me on a preemptible kernel on average before
> I see a runaway kworker spinning in nf_conntrack_cleanup_net_list.
>
> One fix would to change this to:
> if (res < 0) {
> if (ct != last)
> nf_conntrack_get(&ct->ct_general);
>
> But this reference counting isn't needed in the first place.
> We can just store a cookie value instead.
cookie is indeed safer approach.
IIRC, the concern is that cookie could result in providing a bogus
conntrack listing due to object recycling, which is more likely to
happen with SLAB_TYPESAFE_BY_RCU.
nf_ct_get_id() is adding using a random seed to generate the conntrack
id:
u32 nf_ct_get_id(const struct nf_conn *ct)
{
static siphash_aligned_key_t ct_id_seed;
unsigned long a, b, c, d;
net_get_random_once(&ct_id_seed, sizeof(ct_id_seed));
Then, it should be very unlikely that such recycling that leads to
picking up from the wrong conntrack object because two conntrack
objects in the same memory spot will have different id.
