On 8/12/26 2:15 PM, Ilya Maximets wrote:
> On 8/12/26 11:59 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.
>>
>> 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.
>>
>> The teardown path currently waits for the RCU grace period while holding
>> ovs_mutex. Moving synchronize_rcu() outside the mutex would require
>> restructuring the teardown path and is left for a separate change.
>>
>> 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]>
>> ---
> Reviewed-by: Ilya Maximets <[email protected]>
Hmm. LLMs keep complaining about the RCU synchronization under the mutex,
which is a valid concern at the end of a day. So, maybe we should fold
something like this in instead of fixing it later:
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index cc6ea4014c16..e39724390654 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1620,13 +1620,20 @@ static int ovs_ct_limit_init(struct net *net, struct
ovs_net *ovs_net)
return 0;
}
-static void ovs_ct_limit_exit(struct net *net, struct ovs_net *ovs_net)
+static void *ovs_ct_limit_exit_start(struct ovs_net *ovs_net)
{
- const struct ovs_ct_limit_info *info;
+ return rcu_replace_pointer(ovs_net->ct_limit_info, NULL,
+ lockdep_ovsl_is_held());
+}
+
+static void ovs_ct_limit_exit_finish(struct net *net, void *data)
+{
+ const struct ovs_ct_limit_info *info = data;
int i;
- info = rcu_replace_pointer(ovs_net->ct_limit_info, NULL,
- lockdep_ovsl_is_held());
+ if (!info)
+ return;
+
/* Wait for RCU readers to stop using the CT limits. */
synchronize_rcu();
@@ -2029,12 +2036,27 @@ int ovs_ct_init(struct net *net)
#endif
}
-void ovs_ct_exit(struct net *net)
+/* Must be called with ovs_mutex held. Detaches RCU-protected ct_limit_info
+ * and returns an opaque handle for ovs_ct_exit_finish() to complete teardown
+ * after the mutex is released.
+ */
+void *ovs_ct_exit_start(struct net *net __maybe_unused)
+{
+#if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
+ return ovs_ct_limit_exit_start(net_generic(net, ovs_net_id));
+#endif
+ return NULL;
+}
+
+/* Must be called without ovs_mutex held. @data must be the opaque pointer
+ * returned by ovs_ct_exit_start().
+ */
+void ovs_ct_exit_finish(struct net *net, void *data __maybe_unused)
{
struct ovs_net *ovs_net = net_generic(net, ovs_net_id);
#if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
- ovs_ct_limit_exit(net, ovs_net);
+ ovs_ct_limit_exit_finish(net, data);
#endif
if (ovs_net->xt_label)
diff --git a/net/openvswitch/conntrack.h b/net/openvswitch/conntrack.h
index 317e525c8a11..8c4aa7b8a563 100644
--- a/net/openvswitch/conntrack.h
+++ b/net/openvswitch/conntrack.h
@@ -14,7 +14,8 @@ enum ovs_key_attr;
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
int ovs_ct_init(struct net *);
-void ovs_ct_exit(struct net *);
+void *ovs_ct_exit_start(struct net *);
+void ovs_ct_exit_finish(struct net *, void *data);
bool ovs_ct_verify(struct net *, enum ovs_key_attr attr);
int ovs_ct_copy_action(struct net *, const struct nlattr *,
const struct sw_flow_key *, struct sw_flow_actions **,
@@ -40,7 +41,8 @@ void ovs_ct_free_action(const struct nlattr *a);
static inline int ovs_ct_init(struct net *net) { return 0; }
-static inline void ovs_ct_exit(struct net *net) { }
+static inline void *ovs_ct_exit_start(struct net *net) { return NULL; }
+static inline void ovs_ct_exit_finish(struct net *net, void *data) { }
static inline bool ovs_ct_verify(struct net *net, int attr)
{
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ae69b2cabab9..c18dafa920b7 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -2758,15 +2758,16 @@ static void __net_exit list_vports_from_net(struct net
*net, struct net *dnet,
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;
+ void *ct_exit_data;
struct net *net;
LIST_HEAD(head);
ovs_lock();
- ovs_ct_exit(dnet);
+ ct_exit_data = ovs_ct_exit_start(dnet);
list_for_each_entry_safe(dp, dp_next, &ovs_net->dps, list_node)
__dp_destroy(dp);
@@ -2784,6 +2785,8 @@ static void __net_exit ovs_exit_net(struct net *dnet)
ovs_unlock();
+ ovs_ct_exit_finish(dnet, ct_exit_data);
+
cancel_delayed_work_sync(&ovs_net->masks_rebalance);
cancel_work_sync(&ovs_net->dp_notify_work);
}
--
WDYT?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
