Re: [Devel] [PATCH RHEL10 COMMIT] connector: free the per-VE connector state after an RCU grace period

[Konstantin Khorenko <khorenko@virtuozzo.com>]

The commit is pushed to "branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz" and will appear at git@bitbucket.org:openvz/vzkernel.git
after rh10-6.12.0-211.39.1.16.10.vz10
------>
commit 578a77d6c8afd2a6aeb5c25a015fb2d0f22cf198
Author: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
Date:   Tue Aug 25 16:19:52 2026 +0000

    connector: free the per-VE connector state after an RCU grace period
    
    cn_fini_ve() tears down everything the proc event delivery path uses:
    the percpu local_event, the callback device and the kernel netlink
    socket. It only clears ve->cn at the very end. A reader that fetched
    ve->cn right before the teardown dereferences freed memory afterwards:
    the only guard on the delivery path is the ve->cn check in
    proc_event_num_listeners() and nothing keeps the state alive once the
    check has passed.
    
    Today the window is not reachable: the per-VE delivery path is only
    entered for a task alive in this VE, a live task keeps the VE pid
    namespace busy, so zap_pid_ns_processes() -> ve_exit_ns() ->
    cn_fini_ve() cannot run in parallel. A subsequent patch will make
    proc_exit_connector() deliver the exit event with a VE reference
    pinned before exit_notify(), i.e. possibly after the task was reaped
    and stopped pinning the pid namespace, then the teardown can run in
    parallel with the delivery.
    
    Clear ve->cn and wait for an RCU grace period before freeing anything
    reachable from it, so that the delivery path can safely use the state
    it observed within a single RCU read-side critical section (done by
    the next patch). The clearing is done in cn_proc_fini_ve(): it has to
    happen before the first thing the delivery path uses (local_event) is
    freed, and everything else is freed later in cn_fini_ve().
    
    The kernel netlink socket outlives the ve->cn clearing: it is released
    only later in cn_fini_ve(), after this grace period. A message sent
    from the host into the dying VE netns (e.g. via nsenter) can therefore
    still reach cn_call_callback() with ve->cn already NULL, so get_cdev()
    now returns NULL there. Look the callback device up and walk the queue
    under rcu_read_lock() and bail out on NULL, so the receive path is
    covered by the same grace period as the delivery path.
    
    cn_init_ve() publishes ve->cn early with rcu_assign_pointer(), before
    the connector is fully set up, so its error path may already have an
    RCU reader looking at the state. Wait for a grace period there too
    before freeing it, symmetric to cn_proc_fini_ve().
    
    https://virtuozzo.atlassian.net/browse/VSTOR-140421
    Feature: ve: ve generic structures
    Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
    Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 drivers/connector/cn_proc.c   | 11 +++++++++++
 drivers/connector/connector.c | 25 +++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index d4ce1697dd0bb..6095c7def7cea 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -535,5 +535,16 @@ void cn_proc_fini_ve(struct ve_struct *ve)
 				       ve_is_super(ve));
 
 	cn_del_callback_ve(ve, &cn_proc_event_id);
+
+	/*
+	 * Hide the connector state from the proc event delivery path,
+	 * which dereferences ve->cn under rcu_read_lock(), and wait for
+	 * the readers to finish before anything reachable from it is
+	 * freed: the percpu local_event here, the callback device, the
+	 * netlink socket and the state itself in cn_fini_ve().
+	 */
+	RCU_INIT_POINTER(ve->cn, NULL);
+	synchronize_rcu();
+
 	free_percpu(cn->local_event);
 }
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index bf4a83f3f8703..05c281bb321be 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -151,7 +151,7 @@ static int cn_call_callback(struct sk_buff *skb)
 {
 	struct nlmsghdr *nlh;
 	struct cn_callback_entry *i, *cbq = NULL;
-	struct cn_dev *dev = get_cdev(sock_net(skb->sk)->owner_ve);
+	struct cn_dev *dev;
 	struct cn_msg *msg = nlmsg_data(nlmsg_hdr(skb));
 	struct netlink_skb_parms *nsp = &NETLINK_CB(skb);
 	int err = -ENODEV;
@@ -161,6 +161,21 @@ static int cn_call_callback(struct sk_buff *skb)
 	if (nlh->nlmsg_len < NLMSG_HDRLEN + sizeof(struct cn_msg) + msg->len)
 		return -EINVAL;
 
+	/*
+	 * On VE stop cn_proc_fini_ve() clears ve->cn and waits for a grace
+	 * period before the callback device is freed, but the kernel socket
+	 * is released only afterwards, so a message sent from the host into
+	 * the dying VE netns can still get here. Look the device up and walk
+	 * the callback queue under rcu_read_lock(); the callback itself may
+	 * sleep and is kept alive by the refcount taken here.
+	 */
+	rcu_read_lock();
+	dev = get_cdev(sock_net(skb->sk)->owner_ve);
+	if (!dev) {
+		rcu_read_unlock();
+		return -ENODEV;
+	}
+
 	spin_lock_bh(&dev->cbdev->queue_lock);
 	list_for_each_entry(i, &dev->cbdev->queue_list, callback_entry) {
 		if (cn_cb_equal(&i->id.id, &msg->id)) {
@@ -170,6 +185,7 @@ static int cn_call_callback(struct sk_buff *skb)
 		}
 	}
 	spin_unlock_bh(&dev->cbdev->queue_lock);
+	rcu_read_unlock();
 
 	if (cbq != NULL) {
 		cbq->callback(msg, nsp);
@@ -362,7 +378,12 @@ static int cn_init_ve(void *data)
 netlink_release:
 	netlink_kernel_release(dev->nls);
 free_cn:
+	/*
+	 * The pointer was published: wait for the readers before freeing,
+	 * same as cn_proc_fini_ve() does.
+	 */
 	RCU_INIT_POINTER(ve->cn, NULL);
+	synchronize_rcu();
 	kfree(cn);
 	goto net_unlock;
 }
@@ -394,7 +415,7 @@ static void cn_fini_ve(void *data)
 	cn_queue_free_dev(dev->cbdev);
 	netlink_kernel_release(dev->nls);
 
-	RCU_INIT_POINTER(ve->cn, NULL);
+	/* ve->cn was cleared by cn_proc_fini_ve() before the grace period */
 	kfree(cn);
 }
 
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel