Re: [PATCH bpf] bpf: Fix netns reference imbalance in conntrack kfuncs

[sashiko-bot@kernel.org]

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