Re: [PATCH net v8 1/1] openvswitch: Fix CT limit teardown use-after-free

[Ilya Maximets]

On 8/19/26 11:24 AM, Yuqi Xu wrote:
> Packet processing uses CT limit state under RCU, while netns teardown
> frees that state under ovs_mutex. The CT limit pointer was neither removed
> from readers nor protected by a grace period, allowing packet processing to
> dereference the freed state.
> 
> An unprivileged user can trigger this bug from a user and network
> namespace, causing a slab-use-after-free in ovs_ct_execute() when the
> netns is torn down.
> 
> Publish the CT limit pointer through RCU, remove it before teardown, and
> wait for readers before freeing its contents. Keep ovs_mutex around
> individual CT limit updates, and use the RCU read-side lock while GET
> traverses the RCU-protected limit lists.
> 
> Netns teardown detaches the RCU-protected CT limit state in the pernet
> .pre_exit callback while holding ovs_mutex.  The pernet core guarantees an
> RCU grace period between the .pre_exit and .exit callbacks, so the .exit
> callback completes the teardown without adding any extra synchronization.
> 
> The netlink command handlers do not need NULL checks because the userspace
> netlink socket holds an active reference to its network namespace while a
> request is processed. The per-netns exit path therefore cannot run
> concurrently with SET, DEL, or GET for that socket's namespace.
> 
> Fixes: 11efd5cb04a1 ("openvswitch: Support conntrack zone limit")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Vega <vega@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/cover.1784711445.git.xuyuqiabc@xxxxxxxxx
> Assisted-by: Codex:GPT-5.4
> Co-developed-by: Nan Li <tonanli66@xxxxxxxxx>
> Signed-off-by: Nan Li <tonanli66@xxxxxxxxx>
> Signed-off-by: Yuqi Xu <xuyuqiabc@xxxxxxxxx>
> Reviewed-by: Ren Wei <enjou1224z@xxxxxxxxx>
> ---
> 
> Changes in v8:
> 
> - Rebase onto net/main, which now contains the adjacent nf_connlabels
>   leak fix.
> - Move the CT limit detach to the pernet .pre_exit callback and complete
>   the teardown in .exit, relying on the RCU grace period that the pernet
>   core guarantees between the two. Drop the extra synchronize_rcu() and
>   use plain kfree() in the teardown path.
> - v7 Link: https://lore.kernel.org/all/cover.1786936669.git.xuyuqiabc@xxxxxxxxx/

[...]

> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index ded46d993a4e..362e322fb41f 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -2757,17 +2757,24 @@ static void __net_exit list_vports_from_net(struct net *net, struct net *dnet,
>  	}
>  }
>  
> +static void __net_exit ovs_pre_exit_net(struct net *dnet)
> +{
> +	ovs_lock();
> +	ovs_ct_exit_start(dnet);
> +	ovs_unlock();
> +}
> +
>  static void __net_exit ovs_exit_net(struct net *dnet)
>  {
> -	struct datapath *dp, *dp_next;
>  	struct ovs_net *ovs_net = net_generic(dnet, ovs_net_id);
>  	struct vport *vport, *vport_next;
> +	struct datapath *dp, *dp_next;

Shouldn't move these around now that there are no other changes.

>  	struct net *net;
>  	LIST_HEAD(head);
>  
>  	ovs_lock();
>  
> -	ovs_ct_exit(dnet);
> +	ovs_ct_exit_finish(dnet, ovs_net->ct_exit_data);

This will not compile without CONFIG_NETFILTER_CONNCOUNT.
There is also asymmetry here.  The value is not set in this
module, there is no point to pass it from here.  The finish()
function can access it through the dnet pointer.

>  
>  	list_for_each_entry_safe(dp, dp_next, &ovs_net->dps, list_node)
>  		__dp_destroy(dp);
> @@ -2791,6 +2798,7 @@ static void __net_exit ovs_exit_net(struct net *dnet)
>  
>  static struct pernet_operations ovs_net_ops = {
>  	.init = ovs_init_net,
> +	.pre_exit = ovs_pre_exit_net,
>  	.exit = ovs_exit_net,
>  	.id   = &ovs_net_id,
>  	.size = sizeof(struct ovs_net),
> diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
> index 696640e88fa7..446553c51c30 100644
> --- a/net/openvswitch/datapath.h
> +++ b/net/openvswitch/datapath.h
> @@ -164,7 +164,10 @@ struct dp_upcall_info {
>   * Protected by genl_mutex.
>   * @dp_notify_work: A work notifier to handle port unregistering.
>   * @masks_rebalance: A work to periodically optimize flow table caches.
> - * @ct_limit_info: A hash table of conntrack zone connection limits.
> + * @ct_limit_info: Hash table of conntrack zone connection limits. Protected
> + * by RCU; updates and teardown are serialized by ovs_mutex. May be NULL during
> + * netns teardown.
> + * @ct_exit_data: CT limit state detached at .pre_exit, freed at .exit.
>   * @xt_label: Whether connlables are configured for the network or not.
>   */
>  struct ovs_net {
> @@ -172,7 +175,8 @@ struct ovs_net {
>  	struct work_struct dp_notify_work;
>  	struct delayed_work masks_rebalance;
>  #if	IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
> -	struct ovs_ct_limit_info *ct_limit_info;
> +	struct ovs_ct_limit_info __rcu *ct_limit_info;
> +	struct ovs_ct_limit_info *ct_exit_data;

Maybe rename into ct_limit_exit_data, since it is only for the limits
and guarded by the CONFIG_NETFILTER_CONNCOUNT.

>  #endif
>  	bool xt_label;
>  };
>