All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs
@ 2026-07-29 16:31 Chengfeng Ye
  2026-07-29 16:48 ` sashiko-bot
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Chengfeng Ye @ 2026-07-29 16:31 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Stanislav Fomichev, Kumar Kartikeya Dwivedi,
	Lorenzo Bianconi
  Cc: netfilter-devel, coreteam, netdev, linux-kernel, bpf,
	Chengfeng Ye

The opts argument of the BPF conntrack kfuncs can point to a shared map
value.  __bpf_nf_ct_lookup() and __bpf_nf_ct_alloc_entry() read
opts->netns_id separately when acquiring and releasing the network
namespace reference.

The reference imbalance can occur as follows:

  CPU 0                                  CPU 1
  read opts->netns_id (-1)
  skip get_net_ns_by_id()
                                         write opts->netns_id (id)
  read opts->netns_id (id)
  put_net(net) /* no matching get */

The reverse transition leaks the reference.  Repeating the unmatched put
can destroy a live namespace and crash later users.

The kernel reported:

  Oops: general protection fault, probably for non-canonical address
  KASAN: null-ptr-deref in range [0x00000000000000e8-0x00000000000000ef]
  RIP: 0010:bpf_prog_test_run_xdp+0x52c/0x1700
  Call Trace:
   __sys_bpf+0x1662/0x50c0
   __x64_sys_bpf+0x73/0xb0
   do_syscall_64+0xf9/0x540
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
  Kernel panic - not syncing: Fatal exception

Read netns_id once with READ_ONCE() and use that value for validation
and the matching get/put pair.  Each invocation then consistently uses
either the calling namespace or a referenced namespace.

Fixes: aed8ee7feb44 ("net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup")
Fixes: d7e79c97c00c ("net: netfilter: Add kfuncs to allocate and insert CT")
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Please queue this fix for stable kernels.

 net/netfilter/nf_conntrack_bpf.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index f98d1d4b42c3..8b540846f299 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -122,6 +122,7 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
 	struct nf_conntrack_tuple otuple, rtuple;
 	struct nf_conntrack_zone ct_zone;
 	struct nf_conn *ct;
+	s32 netns_id;
 	int err;
 
 	if (!(opts_len == NF_BPF_CT_OPTS_SZ || opts_len == 12))
@@ -134,7 +135,8 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
 			return ERR_PTR(-EINVAL);
 	}
 
-	if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
+	netns_id = READ_ONCE(opts->netns_id);
+	if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
 		return ERR_PTR(-EINVAL);
 
 	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
@@ -147,8 +149,8 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
 	if (err < 0)
 		return ERR_PTR(err);
 
-	if (opts->netns_id >= 0) {
-		net = get_net_ns_by_id(net, opts->netns_id);
+	if (netns_id >= 0) {
+		net = get_net_ns_by_id(net, netns_id);
 		if (unlikely(!net))
 			return ERR_PTR(-ENONET);
 	}
@@ -171,7 +173,7 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
 	__nf_ct_set_timeout(ct, timeout * HZ);
 
 out:
-	if (opts->netns_id >= 0)
+	if (netns_id >= 0)
 		put_net(net);
 
 	return ct;
@@ -186,6 +188,7 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 	struct nf_conntrack_tuple tuple;
 	struct nf_conntrack_zone ct_zone;
 	struct nf_conn *ct;
+	s32 netns_id;
 	int err;
 
 	if (!opts || !bpf_tuple)
@@ -201,7 +204,8 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 	}
 	if (unlikely(opts->l4proto != IPPROTO_TCP && opts->l4proto != IPPROTO_UDP))
 		return ERR_PTR(-EPROTO);
-	if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
+	netns_id = READ_ONCE(opts->netns_id);
+	if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
 		return ERR_PTR(-EINVAL);
 
 	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
@@ -209,8 +213,8 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 	if (err < 0)
 		return ERR_PTR(err);
 
-	if (opts->netns_id >= 0) {
-		net = get_net_ns_by_id(net, opts->netns_id);
+	if (netns_id >= 0) {
+		net = get_net_ns_by_id(net, netns_id);
 		if (unlikely(!net))
 			return ERR_PTR(-ENONET);
 	}
@@ -225,7 +229,7 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 	}
 
 	hash = nf_conntrack_find_get(net, &ct_zone, &tuple);
-	if (opts->netns_id >= 0)
+	if (netns_id >= 0)
 		put_net(net);
 	if (!hash)
 		return ERR_PTR(-ENOENT);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs
  2026-07-29 16:31 [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs Chengfeng Ye
@ 2026-07-29 16:48 ` sashiko-bot
  2026-07-29 17:15 ` bot+bpf-ci
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-29 16:48 UTC (permalink / raw)
  To: Chengfeng Ye; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] TOCTOU races on `bpf_tuple` and `opts->l4proto` allow creation of asymmetrical and corrupted conntrack entries.
--

--- Patch [1]: [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs ---
commit a848b567930cfe1aaae045cb53d2955c9b1029c6
Author: Chengfeng Ye <nicoyip.dev@gmail.com>

bpf: Fix netns reference imbalance in conntrack kfuncs

This commit fixes a netns reference leak in the conntrack BPF kfuncs.
It reads opts->netns_id once using READ_ONCE() to prevent a race
condition where a shared BPF map value is modified concurrently.

> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
> index f98d1d4b42c3d..8b540846f2992 100644
> --- a/net/netfilter/nf_conntrack_bpf.c
> +++ b/net/netfilter/nf_conntrack_bpf.c
[ ... ]
> @@ -134,7 +135,8 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
>  			return ERR_PTR(-EINVAL);
>  	}
>  
> -	if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
> +	netns_id = READ_ONCE(opts->netns_id);
> +	if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
>  		return ERR_PTR(-EINVAL);
>  
>  	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
> @@ -147,8 +149,8 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,

[Severity: High]
This is a pre-existing issue, but since this patch addresses a TOCTOU race
with opts->netns_id, could similar races occur with bpf_tuple and
opts->l4proto?

In net/netfilter/nf_conntrack_bpf.c:__bpf_nf_ct_alloc_entry(),
bpf_nf_ct_tuple_parse() is called twice sequentially:

    err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
                                IP_CT_DIR_ORIGINAL, &otuple);
    if (err < 0)
        return ERR_PTR(err);

    err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
                                IP_CT_DIR_REPLY, &rtuple);

If a BPF program concurrently modifies the shared map value containing
bpf_tuple or opts->l4proto between these two calls, could this create a
conntrack entry with mismatched original and reply tuples?

Could this also result in mismatched L4 protocols, violating core conntrack
invariants?

Additionally, later in the same function, we write directly back to the map
memory:

    if (opts->ct_zone_dir == 0)
        opts->ct_zone_dir = NF_CT_DEFAULT_ZONE_DIR;

Does this inappropriately modify BPF map memory or introduce another
TOCTOU vulnerability if the value is accessed concurrently?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729163141.213611-1-nicoyip.dev@gmail.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs
  2026-07-29 16:31 [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs Chengfeng Ye
  2026-07-29 16:48 ` sashiko-bot
@ 2026-07-29 17:15 ` bot+bpf-ci
  2026-07-30  6:38 ` Emil Tsalapatis
  2026-07-30  8:29 ` [PATCH bpf v2] " Chengfeng Ye
  3 siblings, 0 replies; 7+ messages in thread
From: bot+bpf-ci @ 2026-07-29 17:15 UTC (permalink / raw)
  To: nicoyip.dev, pablo, fw, phil, davem, edumazet, kuba, pabeni,
	horms, ast, daniel, hawk, john.fastabend, sdf, memxor, lorenzo
  Cc: netfilter-devel, coreteam, netdev, linux-kernel, bpf, nicoyip.dev,
	ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

> The kernel reported:
>
>   Oops: general protection fault, probably for non-canonical address
>   KASAN: null-ptr-deref in range [0x00000000000000e8-0x00000000000000ef]
>   RIP: 0010:bpf_prog_test_run_xdp+0x52c/0x1700
>   Call Trace:
>    __sys_bpf+0x1662/0x50c0
>    __x64_sys_bpf+0x73/0xb0
>    do_syscall_64+0xf9/0x540
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>   Kernel panic - not syncing: Fatal exception

This isn't a bug, but could the crash trace be clearer about its
connection to the fix? The trace shows only BPF test-run frames
with no netfilter or conntrack function names visible.

The chain is reconstructible - the extra put_net() frees the
caller's netns, loopback_dev becomes NULL, and bpf_prog_test_run_xdp()
then dereferences it - but a reader has to derive this themselves.

Would it help to add a sentence connecting the trace to the fix, or
include a Reported-by: or Closes: tag for the original report?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30472145413

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs
  2026-07-29 16:31 [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs Chengfeng Ye
  2026-07-29 16:48 ` sashiko-bot
  2026-07-29 17:15 ` bot+bpf-ci
@ 2026-07-30  6:38 ` Emil Tsalapatis
  2026-07-30  8:30   ` Chengfeng Ye
  2026-07-30  8:29 ` [PATCH bpf v2] " Chengfeng Ye
  3 siblings, 1 reply; 7+ messages in thread
From: Emil Tsalapatis @ 2026-07-30  6:38 UTC (permalink / raw)
  To: Chengfeng Ye, Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
	Kumar Kartikeya Dwivedi, Lorenzo Bianconi
  Cc: netfilter-devel, coreteam, netdev, linux-kernel, bpf

On Wed Jul 29, 2026 at 12:31 PM EDT, Chengfeng Ye wrote:
> The opts argument of the BPF conntrack kfuncs can point to a shared map
> value.  __bpf_nf_ct_lookup() and __bpf_nf_ct_alloc_entry() read
> opts->netns_id separately when acquiring and releasing the network
> namespace reference.
>
> The reference imbalance can occur as follows:
>
>   CPU 0                                  CPU 1
>   read opts->netns_id (-1)
>   skip get_net_ns_by_id()
>                                          write opts->netns_id (id)
>   read opts->netns_id (id)
>   put_net(net) /* no matching get */
>
> The reverse transition leaks the reference.  Repeating the unmatched put
> can destroy a live namespace and crash later users.
>
> The kernel reported:
>
>   Oops: general protection fault, probably for non-canonical address
>   KASAN: null-ptr-deref in range [0x00000000000000e8-0x00000000000000ef]
>   RIP: 0010:bpf_prog_test_run_xdp+0x52c/0x1700
>   Call Trace:
>    __sys_bpf+0x1662/0x50c0
>    __x64_sys_bpf+0x73/0xb0
>    do_syscall_64+0xf9/0x540
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>   Kernel panic - not syncing: Fatal exception
>
> Read netns_id once with READ_ONCE() and use that value for validation
> and the matching get/put pair.  Each invocation then consistently uses
> either the calling namespace or a referenced namespace.
>
> Fixes: aed8ee7feb44 ("net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup")
> Fixes: d7e79c97c00c ("net: netfilter: Add kfuncs to allocate and insert CT")
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>

Sashiko is right on this regarding all other fields of opts having the
same issue. While they can't lead to a crash, let's still snapshot them
at the beginning of the function so they don't change from under us.
This is still far from ideal - we may read an inconsistent snapshot of
the struct - but it's backed by BPF program-controlled memory so AFAICT
this is the best we can do.

pw-bot: cr

> ---
> Please queue this fix for stable kernels.
>
>  net/netfilter/nf_conntrack_bpf.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
> index f98d1d4b42c3..8b540846f299 100644
> --- a/net/netfilter/nf_conntrack_bpf.c
> +++ b/net/netfilter/nf_conntrack_bpf.c
> @@ -122,6 +122,7 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
>  	struct nf_conntrack_tuple otuple, rtuple;
>  	struct nf_conntrack_zone ct_zone;
>  	struct nf_conn *ct;
> +	s32 netns_id;
>  	int err;
>  
>  	if (!(opts_len == NF_BPF_CT_OPTS_SZ || opts_len == 12))
> @@ -134,7 +135,8 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
>  			return ERR_PTR(-EINVAL);
>  	}
>  
> -	if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
> +	netns_id = READ_ONCE(opts->netns_id);
> +	if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
>  		return ERR_PTR(-EINVAL);
>  
>  	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
> @@ -147,8 +149,8 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
>  	if (err < 0)
>  		return ERR_PTR(err);
>  
> -	if (opts->netns_id >= 0) {
> -		net = get_net_ns_by_id(net, opts->netns_id);
> +	if (netns_id >= 0) {
> +		net = get_net_ns_by_id(net, netns_id);
>  		if (unlikely(!net))
>  			return ERR_PTR(-ENONET);
>  	}
> @@ -171,7 +173,7 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
>  	__nf_ct_set_timeout(ct, timeout * HZ);
>  
>  out:
> -	if (opts->netns_id >= 0)
> +	if (netns_id >= 0)
>  		put_net(net);
>  
>  	return ct;
> @@ -186,6 +188,7 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
>  	struct nf_conntrack_tuple tuple;
>  	struct nf_conntrack_zone ct_zone;
>  	struct nf_conn *ct;
> +	s32 netns_id;
>  	int err;
>  
>  	if (!opts || !bpf_tuple)
> @@ -201,7 +204,8 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
>  	}
>  	if (unlikely(opts->l4proto != IPPROTO_TCP && opts->l4proto != IPPROTO_UDP))
>  		return ERR_PTR(-EPROTO);
> -	if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
> +	netns_id = READ_ONCE(opts->netns_id);
> +	if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
>  		return ERR_PTR(-EINVAL);
>  
>  	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
> @@ -209,8 +213,8 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
>  	if (err < 0)
>  		return ERR_PTR(err);
>  
> -	if (opts->netns_id >= 0) {
> -		net = get_net_ns_by_id(net, opts->netns_id);
> +	if (netns_id >= 0) {
> +		net = get_net_ns_by_id(net, netns_id);
>  		if (unlikely(!net))
>  			return ERR_PTR(-ENONET);
>  	}
> @@ -225,7 +229,7 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
>  	}
>  
>  	hash = nf_conntrack_find_get(net, &ct_zone, &tuple);
> -	if (opts->netns_id >= 0)
> +	if (netns_id >= 0)
>  		put_net(net);
>  	if (!hash)
>  		return ERR_PTR(-ENOENT);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH bpf v2] bpf: Fix netns reference imbalance in conntrack kfuncs
  2026-07-29 16:31 [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs Chengfeng Ye
                   ` (2 preceding siblings ...)
  2026-07-30  6:38 ` Emil Tsalapatis
@ 2026-07-30  8:29 ` Chengfeng Ye
  2026-07-30  9:52   ` bot+bpf-ci
  3 siblings, 1 reply; 7+ messages in thread
From: Chengfeng Ye @ 2026-07-30  8:29 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Stanislav Fomichev, Kumar Kartikeya Dwivedi,
	Lorenzo Bianconi
  Cc: netfilter-devel, coreteam, netdev, linux-kernel, bpf,
	Chengfeng Ye

The opts argument of the BPF conntrack kfuncs can point to a shared
map value.  __bpf_nf_ct_lookup() and __bpf_nf_ct_alloc_entry() read
opts->netns_id separately when acquiring and releasing the network
namespace reference.

The reference imbalance can occur as follows:

  CPU 0                                  CPU 1
  read opts->netns_id (-1)
  skip get_net_ns_by_id()
                                         write opts->netns_id (id)
  read opts->netns_id (id)
  put_net(net) /* no matching get */

The reverse transition leaks the reference.  Repeating the unmatched put
can destroy a live namespace and crash later users.

The kernel reported:

  Oops: general protection fault, probably for non-canonical address
  KASAN: null-ptr-deref in range [0x00000000000000e8-0x00000000000000ef]
  RIP: 0010:bpf_prog_test_run_xdp+0x52c/0x1700
  Call Trace:
   __sys_bpf+0x1662/0x50c0
   __x64_sys_bpf+0x73/0xb0
   do_syscall_64+0xf9/0x540
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
  Kernel panic - not syncing: Fatal exception

Snapshot every input field of opts with READ_ONCE() before validating or
using it.  The netns_id snapshot keeps the namespace get/put pair
balanced, while the other snapshots keep the remaining options from
changing partway through an invocation.  The individual reads can still
observe an inconsistent combination during a concurrent update, but each
selected field value remains stable for that invocation.

Fixes: aed8ee7feb44 ("net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup")
Fixes: d7e79c97c00c ("net: netfilter: Add kfuncs to allocate and insert CT")
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Changes in v2:
- Snapshot l4proto, ct_zone_id, ct_zone_dir, and the reserved bytes in
  addition to netns_id, as requested in review.
- Rebase onto current bpf/master.

Please queue this fix for stable kernels.

 net/netfilter/nf_conntrack_bpf.c | 76 ++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 24 deletions(-)

diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index f98d1d4b42c3..c3395cb98c00 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -122,42 +122,56 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
 	struct nf_conntrack_tuple otuple, rtuple;
 	struct nf_conntrack_zone ct_zone;
 	struct nf_conn *ct;
+	u16 ct_zone_id;
+	s32 netns_id;
+	u8 ct_zone_dir = 0;
+	u8 reserved[3] = {};
+	u8 l4proto;
 	int err;
 
 	if (!(opts_len == NF_BPF_CT_OPTS_SZ || opts_len == 12))
 		return ERR_PTR(-EINVAL);
+
+	netns_id = READ_ONCE(opts->netns_id);
+	l4proto = READ_ONCE(opts->l4proto);
+	ct_zone_id = READ_ONCE(opts->ct_zone_id);
 	if (opts_len == NF_BPF_CT_OPTS_SZ) {
-		if (opts->reserved[0] || opts->reserved[1] || opts->reserved[2])
+		ct_zone_dir = READ_ONCE(opts->ct_zone_dir);
+		reserved[0] = READ_ONCE(opts->reserved[0]);
+		reserved[1] = READ_ONCE(opts->reserved[1]);
+		reserved[2] = READ_ONCE(opts->reserved[2]);
+		if (reserved[0] || reserved[1] || reserved[2])
 			return ERR_PTR(-EINVAL);
 	} else {
-		if (opts->ct_zone_id)
+		if (ct_zone_id)
 			return ERR_PTR(-EINVAL);
 	}
 
-	if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
+	if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
 		return ERR_PTR(-EINVAL);
 
-	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
+	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, l4proto,
 				    IP_CT_DIR_ORIGINAL, &otuple);
 	if (err < 0)
 		return ERR_PTR(err);
 
-	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
+	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, l4proto,
 				    IP_CT_DIR_REPLY, &rtuple);
 	if (err < 0)
 		return ERR_PTR(err);
 
-	if (opts->netns_id >= 0) {
-		net = get_net_ns_by_id(net, opts->netns_id);
+	if (netns_id >= 0) {
+		net = get_net_ns_by_id(net, netns_id);
 		if (unlikely(!net))
 			return ERR_PTR(-ENONET);
 	}
 
 	if (opts_len == NF_BPF_CT_OPTS_SZ) {
-		if (opts->ct_zone_dir == 0)
-			opts->ct_zone_dir = NF_CT_DEFAULT_ZONE_DIR;
-		nf_ct_zone_init(&ct_zone,
-				opts->ct_zone_id, opts->ct_zone_dir, 0);
+		if (ct_zone_dir == 0) {
+			ct_zone_dir = NF_CT_DEFAULT_ZONE_DIR;
+			opts->ct_zone_dir = ct_zone_dir;
+		}
+		nf_ct_zone_init(&ct_zone, ct_zone_id, ct_zone_dir, 0);
 	} else {
 		ct_zone = nf_ct_zone_dflt;
 	}
@@ -171,7 +185,7 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
 	__nf_ct_set_timeout(ct, timeout * HZ);
 
 out:
-	if (opts->netns_id >= 0)
+	if (netns_id >= 0)
 		put_net(net);
 
 	return ct;
@@ -186,46 +200,60 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
 	struct nf_conntrack_tuple tuple;
 	struct nf_conntrack_zone ct_zone;
 	struct nf_conn *ct;
+	u16 ct_zone_id;
+	s32 netns_id;
+	u8 ct_zone_dir = 0;
+	u8 reserved[3] = {};
+	u8 l4proto;
 	int err;
 
 	if (!opts || !bpf_tuple)
 		return ERR_PTR(-EINVAL);
 	if (!(opts_len == NF_BPF_CT_OPTS_SZ || opts_len == 12))
 		return ERR_PTR(-EINVAL);
+
+	netns_id = READ_ONCE(opts->netns_id);
+	l4proto = READ_ONCE(opts->l4proto);
+	ct_zone_id = READ_ONCE(opts->ct_zone_id);
 	if (opts_len == NF_BPF_CT_OPTS_SZ) {
-		if (opts->reserved[0] || opts->reserved[1] || opts->reserved[2])
+		ct_zone_dir = READ_ONCE(opts->ct_zone_dir);
+		reserved[0] = READ_ONCE(opts->reserved[0]);
+		reserved[1] = READ_ONCE(opts->reserved[1]);
+		reserved[2] = READ_ONCE(opts->reserved[2]);
+		if (reserved[0] || reserved[1] || reserved[2])
 			return ERR_PTR(-EINVAL);
 	} else {
-		if (opts->ct_zone_id)
+		if (ct_zone_id)
 			return ERR_PTR(-EINVAL);
 	}
-	if (unlikely(opts->l4proto != IPPROTO_TCP && opts->l4proto != IPPROTO_UDP))
+	if (unlikely(l4proto != IPPROTO_TCP && l4proto != IPPROTO_UDP))
 		return ERR_PTR(-EPROTO);
-	if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
+	if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
 		return ERR_PTR(-EINVAL);
 
-	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
+	err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, l4proto,
 				    IP_CT_DIR_ORIGINAL, &tuple);
 	if (err < 0)
 		return ERR_PTR(err);
 
-	if (opts->netns_id >= 0) {
-		net = get_net_ns_by_id(net, opts->netns_id);
+	if (netns_id >= 0) {
+		net = get_net_ns_by_id(net, netns_id);
 		if (unlikely(!net))
 			return ERR_PTR(-ENONET);
 	}
 
 	if (opts_len == NF_BPF_CT_OPTS_SZ) {
-		if (opts->ct_zone_dir == 0)
-			opts->ct_zone_dir = NF_CT_DEFAULT_ZONE_DIR;
-		nf_ct_zone_init(&ct_zone,
-				opts->ct_zone_id, opts->ct_zone_dir, 0);
+		if (ct_zone_dir == 0) {
+			ct_zone_dir = NF_CT_DEFAULT_ZONE_DIR;
+			opts->ct_zone_dir = ct_zone_dir;
+		}
+		nf_ct_zone_init(&ct_zone, ct_zone_id, ct_zone_dir, 0);
 	} else {
 		ct_zone = nf_ct_zone_dflt;
 	}
 
 	hash = nf_conntrack_find_get(net, &ct_zone, &tuple);
-	if (opts->netns_id >= 0)
+	if (netns_id >= 0)
 		put_net(net);
 	if (!hash)
 		return ERR_PTR(-ENOENT);
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs
  2026-07-30  6:38 ` Emil Tsalapatis
@ 2026-07-30  8:30   ` Chengfeng Ye
  0 siblings, 0 replies; 7+ messages in thread
From: Chengfeng Ye @ 2026-07-30  8:30 UTC (permalink / raw)
  To: Emil Tsalapatis
  Cc: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Stanislav Fomichev, Kumar Kartikeya Dwivedi,
	Lorenzo Bianconi, netfilter-devel, coreteam, netdev, linux-kernel,
	bpf

On Thu, Jul 30, 2026 at 2:38 PM Emil Tsalapatis <emil@etsalapatis.com> wrote:
> Sashiko is right on this regarding all other fields of opts having the
> same issue. While they can't lead to a crash, let's still snapshot them
> at the beginning of the function so they don't change from under us.
> This is still far from ideal - we may read an inconsistent snapshot of
> the struct - but it's backed by BPF program-controlled memory so AFAICT
> this is the best we can do.
>
> pw-bot: cr

No problem, v2 was just sent.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf v2] bpf: Fix netns reference imbalance in conntrack kfuncs
  2026-07-30  8:29 ` [PATCH bpf v2] " Chengfeng Ye
@ 2026-07-30  9:52   ` bot+bpf-ci
  0 siblings, 0 replies; 7+ messages in thread
From: bot+bpf-ci @ 2026-07-30  9:52 UTC (permalink / raw)
  To: nicoyip.dev, pablo, fw, phil, davem, edumazet, kuba, pabeni,
	horms, ast, daniel, hawk, john.fastabend, sdf, memxor, lorenzo
  Cc: netfilter-devel, coreteam, netdev, linux-kernel, bpf, nicoyip.dev,
	ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 3101 bytes --]

> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
> index f98d1d4b42c3..c3395cb98c00 100644
> --- a/net/netfilter/nf_conntrack_bpf.c
> +++ b/net/netfilter/nf_conntrack_bpf.c
> @@ -122,42 +122,56 @@ __bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
>  	struct nf_conntrack_tuple otuple, rtuple;
>  	struct nf_conntrack_zone ct_zone;
>  	struct nf_conn *ct;
> +	u16 ct_zone_id;
> +	s32 netns_id;
> +	u8 ct_zone_dir = 0;
> +	u8 reserved[3] = {};
> +	u8 l4proto;

This isn't a bug, but would it be worth reordering these declarations
longest-to-shortest to match the netdev reverse-xmas-tree convention
before this goes out again? The block goes from 'struct nf_conn *ct;'
(21 chars) down to 's32 netns_id;' (15), then back up to 'u8 reserved[3]
= {};' (22). The same ordering appears in __bpf_nf_ct_lookup() further
down.

>  	int err;
>
>  	if (!(opts_len == NF_BPF_CT_OPTS_SZ || opts_len == 12))
>  		return ERR_PTR(-EINVAL);
> +
> +	netns_id = READ_ONCE(opts->netns_id);
> +	l4proto = READ_ONCE(opts->l4proto);
> +	ct_zone_id = READ_ONCE(opts->ct_zone_id);
>  	if (opts_len == NF_BPF_CT_OPTS_SZ) {
> -		if (opts->reserved[0] || opts->reserved[1] || opts->reserved[2])
> +		ct_zone_dir = READ_ONCE(opts->ct_zone_dir);
> +		reserved[0] = READ_ONCE(opts->reserved[0]);
> +		reserved[1] = READ_ONCE(opts->reserved[1]);
> +		reserved[2] = READ_ONCE(opts->reserved[2]);
> +		if (reserved[0] || reserved[1] || reserved[2])

This isn't a bug, but since reserved[] is only used by the check on the
next line, could the three READ_ONCE() calls be inlined into the condition
instead of going through a local array? The same block appears in
__bpf_nf_ct_lookup() at the corresponding location.

>  			return ERR_PTR(-EINVAL);
>  	} else {
> -		if (opts->ct_zone_id)
> +		if (ct_zone_id)
>  			return ERR_PTR(-EINVAL);
>  	}

[ ... ]

> @@ -186,46 +200,60 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
>  	struct nf_conntrack_tuple tuple;
>  	struct nf_conntrack_zone ct_zone;
>  	struct nf_conn *ct;
> +	u16 ct_zone_id;
> +	s32 netns_id;
> +	u8 ct_zone_dir = 0;
> +	u8 reserved[3] = {};
> +	u8 l4proto;
>  	int err;
>
>  	if (!opts || !bpf_tuple)
>  		return ERR_PTR(-EINVAL);
>  	if (!(opts_len == NF_BPF_CT_OPTS_SZ || opts_len == 12))
>  		return ERR_PTR(-EINVAL);
> +
> +	netns_id = READ_ONCE(opts->netns_id);
> +	l4proto = READ_ONCE(opts->l4proto);
> +	ct_zone_id = READ_ONCE(opts->ct_zone_id);
>  	if (opts_len == NF_BPF_CT_OPTS_SZ) {
> -		if (opts->reserved[0] || opts->reserved[1] || opts->reserved[2])
> +		ct_zone_dir = READ_ONCE(opts->ct_zone_dir);
> +		reserved[0] = READ_ONCE(opts->reserved[0]);
> +		reserved[1] = READ_ONCE(opts->reserved[1]);
> +		reserved[2] = READ_ONCE(opts->reserved[2]);
> +		if (reserved[0] || reserved[1] || reserved[2])
>  			return ERR_PTR(-EINVAL);

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30527661826

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-30  9:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 16:31 [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs Chengfeng Ye
2026-07-29 16:48 ` sashiko-bot
2026-07-29 17:15 ` bot+bpf-ci
2026-07-30  6:38 ` Emil Tsalapatis
2026-07-30  8:30   ` Chengfeng Ye
2026-07-30  8:29 ` [PATCH bpf v2] " Chengfeng Ye
2026-07-30  9:52   ` bot+bpf-ci

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.