From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vasileios Almpanis Date: Tue, 25 Aug 2026 16:19:52 +0000 Subject: [Devel] [PATCH VZ10 v3 2/4] connector: free the per-VE connector state after an RCU grace period In-Reply-To: <20260825-connectors-v3-0-7b26773876a0@virtuozzo.com> References: <20260825-connectors-v3-0-7b26773876a0@virtuozzo.com> Message-ID: <20260825-connectors-v3-2-7b26773876a0@virtuozzo.com> List-Id: cn_fini_ve() tears down everything the proc event delivery path uses. the percpu local_event, the callback device and the kernel netlink socket. The 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 Signed-off-by: Vasileios Almpanis Feature: ve: ve generic structures --- 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 d4ce1697dd0b..6095c7def7ce 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 bf4a83f3f870..05c281bb321b 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); } -- 2.43.0