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

On 8/17/26 5:32 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 while holding
> ovs_mutex, then completes the teardown - waiting for the RCU grace period
> and freeing the state - after the mutex is released. This keeps the grace
> period wait out of ovs_mutex so that it does not stall concurrent OVS
> users.
> 
> 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: [email protected]
> Reported-by: Vega <[email protected]>
> Link: https://lore.kernel.org/all/[email protected]
> Assisted-by: Codex:GPT-5.4
> Co-developed-by: Nan Li <[email protected]>
> Signed-off-by: Nan Li <[email protected]>
> Signed-off-by: Yuqi Xu <[email protected]>
> Reviewed-by: Ren Wei <[email protected]>
> ---
> 
> Changes in v7:
> 
> - Split CT limit teardown into a start/finish pair so that the RCU grace
>   period wait happens after ovs_mutex is released, keeping the lock from
>   blocking other ovs_mutex users during the wait.
> - v6 Link: https://lore.kernel.org/all/[email protected]/

Need a rebase now that a few other changes in the adjacent code were
accepted to net.

And sashiko raised a few points again:

1. A preexisting nf_connlabels leak - already fixed in net/main.

2. A preexisting default_limit data race - not a big problem, should
   not be fixed here.  A subject for a future cleanup on net-next.

3. The per-netns synchronize_rcu() concern once more.

   For this one actually it seems there is a better solution.  We could
   split the ovs_ct_exit_start() into a .pre_exit hook.  And then run
   the ovs_ct_exit_finish() in the .exit hook.  The pernet_operations
   guarantee that there is RCU synchronization between pre_exit and the
   exit, so we will not be adding any extra synchronization at all.

   We'll need to add ct_exit_data into struct ovs_net though, as we
   can't pass it between two hooks otherwise.  And the comment above
   the ovs_ct_limit_exit_finish() should explain that synchronize_rcu()
   must be executed between the start and the finish.

   While at it, we may also replace kfree_rcu() with kfree() in the
   finish function, if I'm not mistaken.

Best regards, Ilya Maximets.