[PATCH nf] netfilter: nf_conncount: fix zone comparison in tuple dedup

[PATCH nf] netfilter: nf_conncount: fix zone comparison in tuple dedup

[Yizhou Zhao]

The "already exists" dedup logic in __nf_conncount_add() decides
whether a connection has already been counted and can be skipped instead
of incrementing the connlimit count.  It compares the conntrack zone of a
list entry with the zone of the connection being added using
nf_ct_zone_id() and nf_ct_zone_equal(), passing conn->zone.dir or
zone->dir as the direction argument.

Those helpers take enum ip_conntrack_dir values: IP_CT_DIR_ORIGINAL is 0
and IP_CT_DIR_REPLY is 1.  However, zone->dir is a u8 bitmask:
NF_CT_ZONE_DIR_ORIG is 1, NF_CT_ZONE_DIR_REPL is 2 and
NF_CT_DEFAULT_ZONE_DIR is 3.  Passing that bitmask as the enum direction
shifts the meaning of every non-zero value.  An ORIG-only zone passes 1
and is tested as REPLY, while REPL-only and default zones pass 2 or 3 and
test bits beyond the valid direction range.  In those cases
nf_ct_zone_id() can fall back to NF_CT_DEFAULT_ZONE_ID instead of using
the real zone id, so different zones can be treated as equal and dedup
collapses to tuple equality alone.

Do not special-case NF_CT_DEFAULT_ZONE_DIR and do not compare raw zone
ids: that would address only the common bidirectional case and would
bypass the direction-aware NF_CT_DEFAULT_ZONE_ID fallback.  Instead, add
a small conncount-local helper that converts the zone direction bitmask
to a valid enum direction before calling the existing zone helpers.  A
default bidirectional zone contains the ORIG bit, so it naturally maps to
IP_CT_DIR_ORIGINAL; single-direction zones continue to use the existing
nf_ct_zone_id() fallback semantics.

Fixes: 21ba8847f857 ("netfilter: nf_conncount: Fix garbage collection with zones")
Fixes: b36e4523d4d5 ("netfilter: nf_conncount: fix garbage collection confirm race")
Cc: stable@vger.kernel.org
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: Claude-Code:GLM-5.2
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 91582069f6d2..eb3156782405 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -127,6 +127,15 @@ find_or_evict(struct net *net, struct nf_conncount_list *list,
 	return ERR_PTR(-EAGAIN);
 }
 
+static enum ip_conntrack_dir
+nf_conncount_zone_dir(const struct nf_conntrack_zone *zone)
+{
+	if (zone->dir & NF_CT_ZONE_DIR_ORIG)
+		return IP_CT_DIR_ORIGINAL;
+
+	return IP_CT_DIR_REPLY;
+}
+
 static bool get_ct_or_tuple_from_skb(struct net *net,
 				     const struct sk_buff *skb,
 				     u16 l3num,
@@ -211,8 +220,10 @@ static int __nf_conncount_add(struct net *net,
 			/* Not found, but might be about to be confirmed */
 			if (PTR_ERR(found) == -EAGAIN) {
 				if (nf_ct_tuple_equal(&conn->tuple, &tuple) &&
-				    nf_ct_zone_id(&conn->zone, conn->zone.dir) ==
-				    nf_ct_zone_id(zone, zone->dir))
+				    nf_ct_zone_id(&conn->zone,
+						  nf_conncount_zone_dir(&conn->zone)) ==
+				    nf_ct_zone_id(zone,
+						  nf_conncount_zone_dir(zone)))
 					goto out_put; /* already exists */
 			} else {
 				collect++;
@@ -223,7 +234,7 @@ static int __nf_conncount_add(struct net *net,
 		found_ct = nf_ct_tuplehash_to_ctrack(found);
 
 		if (nf_ct_tuple_equal(&conn->tuple, &tuple) &&
-		    nf_ct_zone_equal(found_ct, zone, zone->dir)) {
+		    nf_ct_zone_equal(found_ct, zone, nf_conncount_zone_dir(zone))) {
 			/*
 			 * We should not see tuples twice unless someone hooks
 			 * this into a table without "-p tcp --syn".

--
2.47.3

Re: [PATCH nf] netfilter: nf_conncount: fix zone comparison in tuple dedup

[Florian Westphal]

Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn> wrote:
> The "already exists" dedup logic in __nf_conncount_add() decides
> whether a connection has already been counted and can be skipped instead
> of incrementing the connlimit count.  It compares the conntrack zone of a
> list entry with the zone of the connection being added using
> nf_ct_zone_id() and nf_ct_zone_equal(), passing conn->zone.dir or
> zone->dir as the direction argument.

Right, thats bogus.

> @@ -211,8 +220,10 @@ static int __nf_conncount_add(struct net *net,
>  			/* Not found, but might be about to be confirmed */
>  			if (PTR_ERR(found) == -EAGAIN) {
>  				if (nf_ct_tuple_equal(&conn->tuple, &tuple) &&
> -				    nf_ct_zone_id(&conn->zone, conn->zone.dir) ==
> -				    nf_ct_zone_id(zone, zone->dir))
> +				    nf_ct_zone_id(&conn->zone,
> +						  nf_conncount_zone_dir(&conn->zone)) ==
> +				    nf_ct_zone_id(zone,
> +						  nf_conncount_zone_dir(zone)))

Should this be a simpler:

                                if (nf_ct_tuple_equal(&conn->tuple, &tuple) &&
-                                   nf_ct_zone_id(&conn->zone, conn->zone.dir) ==
-                                   nf_ct_zone_id(zone, zone->dir))
+                                   nf_ct_zone_equal(&conn->zone, &zone), IP_CT_DIR_ORIGINAL)

?

The tuple is always the 'original' direction, so it would follow that
we should not care about reply zone dir.

Also see:
https://sashiko.dev/#/patchset/20260706114820.74006-1-zhaoyz24%40mails.tsinghua.edu.cn

Re: [PATCH nf] netfilter: nf_conncount: fix zone comparison in tuple dedup

[Yizhou Zhao]

Hi Florian,

> On Jul 7, 2026, at 22:58, Florian Westphal <fw@strlen.de> wrote:
> 
> Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn> wrote:
>> The "already exists" dedup logic in __nf_conncount_add() decides
>> whether a connection has already been counted and can be skipped instead
>> of incrementing the connlimit count.  It compares the conntrack zone of a
>> list entry with the zone of the connection being added using
>> nf_ct_zone_id() and nf_ct_zone_equal(), passing conn->zone.dir or
>> zone->dir as the direction argument.
> 
> Right, thats bogus.
> 
>> @@ -211,8 +220,10 @@ static int __nf_conncount_add(struct net *net,
>> /* Not found, but might be about to be confirmed */
>> if (PTR_ERR(found) == -EAGAIN) {
>> if (nf_ct_tuple_equal(&conn->tuple, &tuple) &&
>> -    nf_ct_zone_id(&conn->zone, conn->zone.dir) ==
>> -    nf_ct_zone_id(zone, zone->dir))
>> +    nf_ct_zone_id(&conn->zone,
>> +  nf_conncount_zone_dir(&conn->zone)) ==
>> +    nf_ct_zone_id(zone,
>> +  nf_conncount_zone_dir(zone)))
> 
> Should this be a simpler:
> 
>                                if (nf_ct_tuple_equal(&conn->tuple, &tuple) &&
> -                                   nf_ct_zone_id(&conn->zone, conn->zone.dir) ==
> -                                   nf_ct_zone_id(zone, zone->dir))
> +                                   nf_ct_zone_equal(&conn->zone, &zone), IP_CT_DIR_ORIGINAL)
> 
> ?
> 
> The tuple is always the 'original' direction, so it would follow that
> we should not care about reply zone dir.
> 
> Also see:
> https://sashiko.dev/#/patchset/20260706114820.74006-1-zhaoyz24%40mails.tsinghua.edu.cn

Thank you for pointing out this.

We have published a v2 patch following your suggestions:

https://lore.kernel.org/netfilter-devel/20260708052730.18354-1-zhaoyz24@mails.tsinghua.edu.cn/


Thanks,
Yizhou