OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 help / color / mirror / Atom feed
* [Devel] [PATCH VZ10 v3 0/4] connector: do not lose exit events for in-CT listeners
@ 2026-08-25 16:19 Vasileios Almpanis
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 1/4] connector: annotate ve->cn with __rcu Vasileios Almpanis
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Vasileios Almpanis @ 2026-08-25 16:19 UTC (permalink / raw)


The proc connector reports the exit event after exit_notify(), so the
parent may already have reaped the exiting task. release_task() ->
exit_ve_namespace() then resets tsk->task_ve to ve0, and if that wins
the race the event is delivered to the host listeners only. A listener
inside the container never sees the exit event. LTP catches this as the
flaky cn_pec exit-test failure causing the test to hang for as long as 8
hours on GCOV kernels.

Patch 4 fixes this by pinning the task's VE for the exit notification
before exit_notify(). This means the delivery path can now run against
a VE whose last task is already reaped, i.e. concurrently with the
per-VE connector teardown, which was impossible before. Patches 1-3
prepare for that: annotate ve->cn with __rcu, free the per-VE connector
state only after an RCU grace period, and deliver per-VE events under
rcu_read_lock().

Testing:
KASAN, PROVE_LOCKING/PROVE_RCU, DEBUG_OBJECTS, kmemleak=on; HCI VM with one
container):
 - LTP cn_pec inside a CT: unpatched kernel loses exit events in 3 of
   10 runs (-n 50); patched kernel passes 30 of 30 runs.
 - Teardown stress: 15 cycles of an active in-CT pec_listener plus a
   200-process exit storm with vzctl stop/start while events are in
   flight: no KASAN/lockdep/RCU splats, kmemleak scans report 0
   unreferenced objects.

https://virtuozzo.atlassian.net/browse/VSTOR-140421

---
Changes in v3:
- Patch 2: also make the netlink receive path safe against the VE stop.
  cn_call_callback() can be reached by a host task in the CT netns
  (e.g. nsenter) after ve->cn is cleared but before the kernel socket
  is released in cn_fini_ve(); look up the callback device and walk the
  queue under rcu_read_lock() and bail out on NULL (Konstantin).
- Patch 2: wait for an RCU grace period in the cn_init_ve() error path
  too before freeing ve->cn, since it is published early with
  rcu_assign_pointer(); symmetric to cn_proc_fini_ve(). This also
  covers the early-publish concern raised on patch 1.
- Link to v2: https://lists.openvz.org/pipermail/devel/2026-August/084616.html

Changes in v2:
- Turned the single patch into a series: with the exit event now
  delivered for a possibly already-reaped task, the per-VE delivery
  path can race with the connector teardown in cn_fini_ve(). New
  patches 1-3 make that safe: annotate ve->cn with __rcu, free the
  per-VE connector state only after an RCU grace period, and run the
  delivery under rcu_read_lock().
- cn_proc.h: forward-declare struct ve_struct instead of including
  linux/ve.h.
- Link to v1: https://lists.openvz.org/pipermail/devel/2026-August/084581.html

---
Vasileios Almpanis (4):
      connector: annotate ve->cn with __rcu
      connector: free the per-VE connector state after an RCU grace period
      connector: deliver per-VE proc events under an RCU read lock
      proc connector: pin task VE for the exit event notification

 drivers/connector/cn_proc.c   | 105 ++++++++++++++++++++++++++++++++++--------
 drivers/connector/connector.c |  70 ++++++++++++++++++++++------
 include/linux/cn_proc.h       |   8 +++-
 include/linux/ve.h            |   2 +-
 kernel/exit.c                 |   5 +-
 5 files changed, 153 insertions(+), 37 deletions(-)
---
base-commit: 1e648e17487b16486de22cefcd27672a454a1c3e
change-id: 20260813-connectors-3571be4ae57b

-- 
Best regards, Vasileios Almpanis
Software Developer, Virtuozzo.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Devel] [PATCH VZ10 v3 1/4] connector: annotate ve->cn with __rcu
  2026-08-25 16:19 [Devel] [PATCH VZ10 v3 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
@ 2026-08-25 16:19 ` Vasileios Almpanis
  2026-08-26 13:09   ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 2/4] connector: free the per-VE connector state after an RCU grace period Vasileios Almpanis
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Vasileios Almpanis @ 2026-08-25 16:19 UTC (permalink / raw)


The following patches will clear ve->cn on container stop and free the
per-VE connector state only after an RCU grace period, so that the
proc event delivery path can use it under rcu_read_lock() even when
the reported task no longer pins the VE.

Prepare for that: annotate ve->cn with __rcu and switch all accesses
to the RCU accessors so sparse can verify the protocol. All current
users run either in the context of a task alive in the VE or from
the VE start/stop hooks under ve->op_sem (or on ve0), so plain readers
use rcu_dereference_check(ve->cn, 1) with a comment and the start/stop
hooks use rcu_dereference_protected() with the proper lockdep
condition.

No functional change intended

https://virtuozzo.atlassian.net/browse/VSTOR-140421
Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Feature: ve: ve generic structures
---
 drivers/connector/cn_proc.c   | 48 +++++++++++++++++++++++++++++++------------
 drivers/connector/connector.c | 43 +++++++++++++++++++++++++-------------
 include/linux/ve.h            |  2 +-
 3 files changed, 65 insertions(+), 28 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index d41c8aca1866..d4ce1697dd0b 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -84,6 +84,8 @@ static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data)
 
 static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 {
+	/* See the comment in proc_event_num_listeners() */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 	struct local_event *le_ptr;
 	__u32 filter_data[2];
 
@@ -93,9 +95,9 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	 * so be of the safe side.
 	 */
 	BUILD_BUG_ON(offsetof(struct local_event, lock) != 0);
-	local_lock(&ve->cn->local_event->lock);
+	local_lock(&cn->local_event->lock);
 
-	le_ptr = this_cpu_ptr(ve->cn->local_event);
+	le_ptr = this_cpu_ptr(cn->local_event);
 	msg->seq = le_ptr->count++;
 	((struct proc_event *)msg->data)->cpu = smp_processor_id();
 
@@ -116,7 +118,7 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	cn_netlink_send_mult_ve(ve, msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
 				cn_filter, (void *)filter_data);
 
-	local_unlock(&ve->cn->local_event->lock);
+	local_unlock(&cn->local_event->lock);
 }
 
 static struct cn_msg *cn_msg_fill(__u8 *buffer, struct ve_struct *ve,
@@ -147,8 +149,15 @@ static struct cn_msg *cn_msg_fill(__u8 *buffer, struct ve_struct *ve,
 
 static int proc_event_num_listeners(struct ve_struct *ve)
 {
-	if (ve->cn)
-		return atomic_read(&ve->cn->proc_event_num_listeners);
+	/*
+	 * Callers not under rcu_read_lock() are pinned by a live task
+	 * of this VE (or run on ve0 whose connector state lives as long
+	 * as the connector itself).
+	 */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
+
+	if (cn)
+		return atomic_read(&cn->proc_event_num_listeners);
 	return 0;
 }
 
@@ -414,6 +423,8 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
 	enum proc_cn_event ev_type = 0;
 	int err = 0, initial = 0;
 	struct sock *sk = NULL;
+	/* current is a live task of this VE, it cannot be stopped under us */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 
 	/* 
 	 * Events are reported with respect to the initial pid
@@ -467,11 +478,11 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
 	switch (mc_op) {
 	case PROC_CN_MCAST_LISTEN:
 		if (initial || (prev_mc_op != PROC_CN_MCAST_LISTEN))
-			atomic_inc(&ve->cn->proc_event_num_listeners);
+			atomic_inc(&cn->proc_event_num_listeners);
 		break;
 	case PROC_CN_MCAST_IGNORE:
 		if (!initial && (prev_mc_op != PROC_CN_MCAST_IGNORE))
-			atomic_dec(&ve->cn->proc_event_num_listeners);
+			atomic_dec(&cn->proc_event_num_listeners);
 		((struct proc_input *)(sk->sk_user_data))->event_type =
 			PROC_EVENT_NONE;
 		break;
@@ -488,13 +499,18 @@ int cn_proc_init_ve(struct ve_struct *ve)
 {
 	int err, cpu;
 	struct local_event *le_ptr;
+	struct cn_private *cn;
 
-	ve->cn->local_event = alloc_percpu(struct local_event);
-	if (!ve->cn->local_event)
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+
+	cn->local_event = alloc_percpu(struct local_event);
+	if (!cn->local_event)
 		return -ENOMEM;
 
 	for_each_possible_cpu(cpu) {
-		le_ptr = per_cpu_ptr(ve->cn->local_event, cpu);
+		le_ptr = per_cpu_ptr(cn->local_event, cpu);
 		local_lock_init(&le_ptr->lock);
 	}
 
@@ -503,15 +519,21 @@ int cn_proc_init_ve(struct ve_struct *ve)
 				  &cn_proc_mcast_ctl);
 	if (err) {
 		pr_warn("VE#%d: cn_proc failed to register\n", ve->veid);
-		free_percpu(ve->cn->local_event);
+		free_percpu(cn->local_event);
 		return err;
 	}
-	atomic_set(&ve->cn->proc_event_num_listeners, 0);
+	atomic_set(&cn->proc_event_num_listeners, 0);
 	return 0;
 }
 
 void cn_proc_fini_ve(struct ve_struct *ve)
 {
+	struct cn_private *cn;
+
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+
 	cn_del_callback_ve(ve, &cn_proc_event_id);
-	free_percpu(ve->cn->local_event);
+	free_percpu(cn->local_event);
 }
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 39a697803d0f..bf4a83f3f870 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -29,7 +29,13 @@ MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_CONNECTOR);
 
 static struct cn_dev *get_cdev(struct ve_struct *ve)
 {
-	return &ve->cn->cdev;
+	/*
+	 * Callers not under rcu_read_lock() are pinned by a live task
+	 * of this VE or run from the VE start/stop hooks.
+	 */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
+
+	return cn ? &cn->cdev : NULL;
 }
 
 /*
@@ -230,12 +236,13 @@ int cn_add_callback_ve(struct ve_struct *ve,
 		       void (*callback)(struct cn_msg *,
 					struct netlink_skb_parms *))
 {
-	struct cn_dev *dev = get_cdev(ve);
+	/* See the comment in get_cdev() */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 
-	if (!ve->cn->cn_already_initialized)
+	if (!cn || !cn->cn_already_initialized)
 		return -EAGAIN;
 
-	return cn_queue_add_callback(dev->cbdev, name, id, callback);
+	return cn_queue_add_callback(cn->cdev.cbdev, name, id, callback);
 }
 
 /*
@@ -297,6 +304,7 @@ static int __maybe_unused cn_proc_show(struct seq_file *m, void *v)
 static int cn_init_ve(void *data)
 {
 	struct ve_struct *ve = data;
+	struct cn_private *cn;
 	struct cn_dev *dev;
 	struct netlink_kernel_cfg cfg = {
 		.groups	= CN_NETLINK_USERS + 0xf,
@@ -308,11 +316,12 @@ static int cn_init_ve(void *data)
 	struct net *net;
 	int err;
 
-	ve->cn = kzalloc(sizeof(*ve->cn), GFP_KERNEL);
-	if (!ve->cn)
+	cn = kzalloc(sizeof(*cn), GFP_KERNEL);
+	if (!cn)
 		return -ENOMEM;
+	rcu_assign_pointer(ve->cn, cn);
 
-	dev = &ve->cn->cdev;
+	dev = &cn->cdev;
 
 	/*
 	 * This is a hook, hooks are called under a single lock, so ve_nsproxy will
@@ -331,7 +340,7 @@ static int cn_init_ve(void *data)
 		goto netlink_release;
 	}
 
-	ve->cn->cn_already_initialized = 1;
+	cn->cn_already_initialized = 1;
 
 	if (!proc_ve_create_single("connector", S_IRUGO, net->proc_net,
 				cn_proc_show)) {
@@ -353,18 +362,24 @@ static int cn_init_ve(void *data)
 netlink_release:
 	netlink_kernel_release(dev->nls);
 free_cn:
-	kfree(ve->cn);
-	ve->cn = NULL;
+	RCU_INIT_POINTER(ve->cn, NULL);
+	kfree(cn);
 	goto net_unlock;
 }
 
 static void cn_fini_ve(void *data)
 {
 	struct ve_struct *ve = data;
-	struct cn_dev *dev = get_cdev(ve);
+	struct cn_private *cn;
+	struct cn_dev *dev;
 	struct net *net;
 
-	ve->cn->cn_already_initialized = 0;
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+	dev = &cn->cdev;
+
+	cn->cn_already_initialized = 0;
 
 	cn_proc_fini_ve(ve);
 
@@ -379,8 +394,8 @@ static void cn_fini_ve(void *data)
 	cn_queue_free_dev(dev->cbdev);
 	netlink_kernel_release(dev->nls);
 
-	kfree(ve->cn);
-	ve->cn = NULL;
+	RCU_INIT_POINTER(ve->cn, NULL);
+	kfree(cn);
 }
 
 #ifdef CONFIG_VE
diff --git a/include/linux/ve.h b/include/linux/ve.h
index 46e67d00cf78..2dc93e42784b 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -102,7 +102,7 @@ struct ve_struct {
 	char			core_pattern[CORENAME_MAX_SIZE];
 #endif
 #ifdef CONFIG_CONNECTOR
-	struct cn_private	*cn;
+	struct cn_private __rcu	*cn;
 #endif
 
 	struct kthread_worker	*kthreadd_worker;

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Devel] [PATCH VZ10 v3 2/4] connector: free the per-VE connector state after an RCU grace period
  2026-08-25 16:19 [Devel] [PATCH VZ10 v3 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 1/4] connector: annotate ve->cn with __rcu Vasileios Almpanis
@ 2026-08-25 16:19 ` Vasileios Almpanis
  2026-08-26 13:09   ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 3/4] connector: deliver per-VE proc events under an RCU read lock Vasileios Almpanis
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 4/4] proc connector: pin task VE for the exit event notification Vasileios Almpanis
  3 siblings, 1 reply; 9+ messages in thread
From: Vasileios Almpanis @ 2026-08-25 16:19 UTC (permalink / raw)


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 <vasileios.almpanis@virtuozzo.com>

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Devel] [PATCH VZ10 v3 3/4] connector: deliver per-VE proc events under an RCU read lock
  2026-08-25 16:19 [Devel] [PATCH VZ10 v3 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 1/4] connector: annotate ve->cn with __rcu Vasileios Almpanis
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 2/4] connector: free the per-VE connector state after an RCU grace period Vasileios Almpanis
@ 2026-08-25 16:19 ` Vasileios Almpanis
  2026-08-26 13:09   ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 4/4] proc connector: pin task VE for the exit event notification Vasileios Almpanis
  3 siblings, 1 reply; 9+ messages in thread
From: Vasileios Almpanis @ 2026-08-25 16:19 UTC (permalink / raw)


proc_event_connector_ve() dereferences ve->cn several times and
fill_exit_event() dereferences ve->ve_nsproxy assuming the VE cannot
stop in the middle of the delivery. This holds while the reported task
is alive in the VE: a live task keeps the VE pid namespace busy, so
zap_pid_ns_processes() -> ve_exit_ns() cannot start.

The next patch makes proc_exit_connector() deliver the exit event with
a VE reference pinned before exit_notify(). Once the task is reaped,
its pid no longer keeps the pid namespace busy: the container init may
be woken up by free_pid() from release_task(), finish
zap_pid_ns_processes() and run ve_exit_ns() while the exit event is
still being delivered:

    cpu0: exiting task                cpu1: container init
    do_exit()
      exit_notify()
        release_task()
          free_pid()  ------------->  wakes zap_pid_ns_processes()
      proc_exit_connector()           ve_exit_ns()
        proc_event_connector_ve()       cn_fini_ve()      /* ve->cn */
          fill_exit_event()             ve_drop_context() /* ve_nsproxy */
            ve->ve_nsproxy->...

Deliver the event under rcu_read_lock() and recheck the pointers: the
previous patch guarantees everything reachable from ve->cn stays alive
for the whole read-side critical section once observed, and
ve_drop_context() already waits for a grace period before dropping
ve_nsproxy. Bail out if the VE is being stopped: its listeners are
dead anyway, there is nobody to deliver to.

The whole delivery path runs with GFP_NOWAIT and never sleeps, so it
is legal inside an RCU read-side critical section.

https://virtuozzo.atlassian.net/browse/VSTOR-140421

Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
Feature: ve: ve generic structures
---
 drivers/connector/cn_proc.c   | 36 +++++++++++++++++++++++++++++++++---
 drivers/connector/connector.c |  4 ++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 6095c7def7ce..608430808548 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -89,6 +89,10 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	struct local_event *le_ptr;
 	__u32 filter_data[2];
 
+	/* The VE is being stopped, so are its listeners: nothing to do */
+	if (!cn)
+		return;
+
 	/*
 	 * The following hack with local_event->lock address works only
 	 * till the "lock" is the first field in the local_event struct,
@@ -172,15 +176,27 @@ static void proc_event_connector_ve(struct task_struct *task,
 	struct cn_msg *msg;
 	__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
 
+	/*
+	 * The exit event may be delivered when the reported task no
+	 * longer pins the VE (see proc_exit_connector()), so the VE may
+	 * be stopping concurrently. cn_proc_fini_ve() waits for an RCU
+	 * grace period before the connector state is freed, take the RCU
+	 * read lock to make the state observed here stay valid for the
+	 * whole delivery. The path below never sleeps (GFP_NOWAIT).
+	 */
+	rcu_read_lock();
+
 	if (proc_event_num_listeners(ve) < 1)
-		return;
+		goto out_unlock;
 
 	msg = cn_msg_fill(buffer, ve, task, what, cookie, fill_event);
 	if (!msg)
-		return;
+		goto out_unlock;
 
 	/*  If cn_netlink_send() failed, the data is not sent */
 	send_msg_ve(ve, msg);
+out_unlock:
+	rcu_read_unlock();
 }
 
 static void proc_event_connector(struct task_struct *task,
@@ -350,9 +366,23 @@ void proc_coredump_connector(struct task_struct *task)
 static bool fill_exit_event(struct proc_event *ev, struct ve_struct *ve,
 			    struct task_struct *task, long cookie_pids)
 {
-	struct pid_namespace *pid_ns = ve->ve_nsproxy->pid_ns_for_children;
+	struct pid_namespace *pid_ns;
 	struct task_struct *parent;
 	struct pids *pids = (struct pids *)cookie_pids;
+	struct nsproxy *nsproxy;
+
+	/*
+	 * Unlike all other events, the exit event may be delivered after
+	 * the task was reaped, when nothing keeps the VE pid namespace
+	 * busy anymore and the VE may be stopping concurrently.
+	 * ve_drop_context() clears ve_nsproxy and waits for an RCU grace
+	 * period before dropping it; we are called under rcu_read_lock().
+	 * The VE is dead, so are its listeners: skip the event.
+	 */
+	nsproxy = rcu_dereference(ve->ve_nsproxy);
+	if (!nsproxy)
+		return false;
+	pid_ns = nsproxy->pid_ns_for_children;
 
 	ev->event_data.exit.process_pid = pid_nr_ns(pids->pid, pid_ns);
 	ev->event_data.exit.process_tgid = pid_nr_ns(pids->tgid, pid_ns);
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 05c281bb321b..5597801c4af2 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -79,6 +79,10 @@ int cn_netlink_send_mult_ve(struct ve_struct *ve, struct cn_msg *msg, u16 len,
 	u32 group = 0;
 	int found = 0;
 
+	/* The VE is being stopped, see proc_event_connector_ve() */
+	if (!dev)
+		return -ENODEV;
+
 	if (portid || __group) {
 		group = __group;
 	} else {

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Devel] [PATCH VZ10 v3 4/4] proc connector: pin task VE for the exit event notification
  2026-08-25 16:19 [Devel] [PATCH VZ10 v3 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
                   ` (2 preceding siblings ...)
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 3/4] connector: deliver per-VE proc events under an RCU read lock Vasileios Almpanis
@ 2026-08-25 16:19 ` Vasileios Almpanis
  2026-08-26 13:09   ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko
  3 siblings, 1 reply; 9+ messages in thread
From: Vasileios Almpanis @ 2026-08-25 16:19 UTC (permalink / raw)


The exit event is reported after exit_notify(), so the parent might have
been woken up and reaped the exiting task via wait() -> release_task()
-> exit_ve_namespace(), which resets tsk->task_ve to ve0. If that wins
the race against the exiting task then the event will only be delivered
to the host VE listeners and the in-VE listeners will be skipped. For a
task inside a container this means the listener never receives an exit
event.

The issue is caught by the LTP suite_kernel_misc.exec.cn_pec_sh test:
pec_listener terminates upon receiving the exit event of the pid given
via -p. When the lost exit event is a child's one, the test fails:

  cn_pec 3 TFAIL: Event was not detected by the event listener:
                  exit pid: 58388 exit_code: 0 exit_signal: 17

and when it is the event generator's own exit event, the listener
polls the netlink socket forever and the test hangs until the LTP
timeout kills it (~8h on coverage kernels):

22:51:37 cn_pec 2 TINFO: Testing exec event (nevents=10)
07:11:37 Test timed out, sending SIGTERM!

The race was captured using kprobes on the connector send path:
  p:cnp/pexit proc_exit_connector task=$arg1:x64
  p:cnp/vexit exit_ve_namespace task=$arg1:x64
  p:cnp/pevcve proc_event_connector_ve what=$arg3:u32
  r:cnp/cnsend cn_netlink_send_mult_ve ret=$retval:s64

A normal exit looks like:
  pexit -> pevcve(ve) -> cnsend ret=0 -> pevcve(ve0)

The lost event (task 0xffff89a506d73980 is the exiting child, reaped by
its parent pid 465820 in between):

 465831 [001] pexit: (proc_exit_connector) task=0xffff89a506d73980
 465820 [002] vexit: (exit_ve_namespace) task=0xffff89a506d73980
 465831 [001] pevcve: (proc_event_connector_ve) what=2147483648

Only one proc_event_connector_ve() call fires (ve0, no listeners) and
cn_netlink_send_mult_ve() is never reached: the event is dropped.

This is the same race window that commit c565cc211694 ("proc
connector: report proper pid/tgid of an exited process") closed for
the task pid/tgid, but nothing pins the VE.

Reproducer (fails within ~50 iterations in a CT on a coverage kernel):

  cd /opt/ltp/testcases/bin/
  export PATH=$PATH:/opt/ltp/testcases/bin
  for i in $(seq 1 1024); do cn_pec.sh >/dev/null 2>&1 || break; done

Solve this the same way. Pin the VE in do_exit() before exit_notify()
is called and use it in proc_exit_connector() instead of re-reading
task->task_ve.

https://virtuozzo.atlassian.net/browse/VSTOR-140421
Fixes: 95fa2f096b72 ("ve: Introduce VE namespace")
Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Feature: ve: ve generic structures
---
 drivers/connector/cn_proc.c | 10 ++++++++--
 include/linux/cn_proc.h     |  8 ++++++--
 kernel/exit.c               |  5 ++++-
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 608430808548..8d8c538cc3cd 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -401,9 +401,15 @@ static bool fill_exit_event(struct proc_event *ev, struct ve_struct *ve,
 	return true;
 }
 
-void proc_exit_connector(struct task_struct *task, struct pids *pids)
+void proc_exit_connector(struct task_struct *task, struct pids *pids,
+			 struct ve_struct *ve)
 {
-	proc_event_connector(task, PROC_EVENT_EXIT, (long)pids, fill_exit_event);
+	if (!ve_is_super(ve))
+		proc_event_connector_ve(task, ve, PROC_EVENT_EXIT, (long)pids,
+					fill_exit_event);
+
+	proc_event_connector_ve(task, get_ve0(), PROC_EVENT_EXIT, (long)pids,
+				fill_exit_event);
 }
 
 /*
diff --git a/include/linux/cn_proc.h b/include/linux/cn_proc.h
index 9701c13d82df..aa606192b5c2 100644
--- a/include/linux/cn_proc.h
+++ b/include/linux/cn_proc.h
@@ -19,6 +19,8 @@
 
 #include <uapi/linux/cn_proc.h>
 
+struct ve_struct;
+
 /*
  * The struct is used solely for pinning task pids for proc connector
  * notification on process exit.
@@ -36,7 +38,8 @@ void proc_sid_connector(struct task_struct *task);
 void proc_ptrace_connector(struct task_struct *task, int which_id);
 void proc_comm_connector(struct task_struct *task);
 void proc_coredump_connector(struct task_struct *task);
-void proc_exit_connector(struct task_struct *task, struct pids *pids);
+void proc_exit_connector(struct task_struct *task, struct pids *pids,
+			 struct ve_struct *ve);
 #else
 static inline void proc_fork_connector(struct task_struct *task)
 {}
@@ -61,7 +64,8 @@ static inline void proc_ptrace_connector(struct task_struct *task,
 static inline void proc_coredump_connector(struct task_struct *task)
 {}
 
-static inline void proc_exit_connector(struct task_struct *task, struct pids *pids)
+static inline void proc_exit_connector(struct task_struct *task, struct pids *pids,
+				       struct ve_struct *ve)
 {}
 #endif	/* CONFIG_PROC_EVENTS */
 #endif	/* CN_PROC_H */
diff --git a/kernel/exit.c b/kernel/exit.c
index 448a734270a7..94d9bddae2b8 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -936,6 +936,7 @@ void __noreturn do_exit(long code)
 	struct task_struct *tsk = current;
 	int group_dead;
 	struct pids pids;
+	struct ve_struct *ve;
 
 	WARN_ON(irqs_disabled());
 
@@ -1021,8 +1022,10 @@ void __noreturn do_exit(long code)
 	exit_tasks_rcu_start();
 	pids.pid = get_pid(task_pid(tsk));
 	pids.tgid = get_pid(task_tgid(tsk));
+	ve = get_task_ve(tsk);
 	exit_notify(tsk, group_dead);
-	proc_exit_connector(tsk, &pids);
+	proc_exit_connector(tsk, &pids, ve);
+	put_ve(ve);
 	put_pid(pids.tgid);
 	put_pid(pids.pid);
 	mpol_put_task_policy(tsk);

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Devel] [PATCH RHEL10 COMMIT] connector: annotate ve->cn with __rcu
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 1/4] connector: annotate ve->cn with __rcu Vasileios Almpanis
@ 2026-08-26 13:09   ` Konstantin Khorenko
  0 siblings, 0 replies; 9+ messages in thread
From: Konstantin Khorenko @ 2026-08-26 13:09 UTC (permalink / raw)
  To: Vasileios Almpanis; +Cc: OpenVZ devel

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 644625e415b34c9e1be8adeadf4e17f72f586ee1
Author: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
Date:   Tue Aug 25 16:19:51 2026 +0000

    connector: annotate ve->cn with __rcu
    
    The following patches will clear ve->cn on container stop and free the
    per-VE connector state only after an RCU grace period, so that the
    proc event delivery path can use it under rcu_read_lock() even when
    the reported task no longer pins the VE.
    
    Prepare for that: annotate ve->cn with __rcu and switch all accesses
    to the RCU accessors so sparse can verify the protocol. All current
    users run either in the context of a task alive in the VE or from
    the VE start/stop hooks under ve->op_sem (or on ve0), so plain readers
    use rcu_dereference_check(ve->cn, 1) with a comment and the start/stop
    hooks use rcu_dereference_protected() with the proper lockdep
    condition.
    
    No functional change intended
    
    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   | 48 +++++++++++++++++++++++++++++++------------
 drivers/connector/connector.c | 43 +++++++++++++++++++++++++-------------
 include/linux/ve.h            |  2 +-
 3 files changed, 65 insertions(+), 28 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index d41c8aca1866b..d4ce1697dd0bb 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -84,6 +84,8 @@ static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data)
 
 static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 {
+	/* See the comment in proc_event_num_listeners() */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 	struct local_event *le_ptr;
 	__u32 filter_data[2];
 
@@ -93,9 +95,9 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	 * so be of the safe side.
 	 */
 	BUILD_BUG_ON(offsetof(struct local_event, lock) != 0);
-	local_lock(&ve->cn->local_event->lock);
+	local_lock(&cn->local_event->lock);
 
-	le_ptr = this_cpu_ptr(ve->cn->local_event);
+	le_ptr = this_cpu_ptr(cn->local_event);
 	msg->seq = le_ptr->count++;
 	((struct proc_event *)msg->data)->cpu = smp_processor_id();
 
@@ -116,7 +118,7 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	cn_netlink_send_mult_ve(ve, msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
 				cn_filter, (void *)filter_data);
 
-	local_unlock(&ve->cn->local_event->lock);
+	local_unlock(&cn->local_event->lock);
 }
 
 static struct cn_msg *cn_msg_fill(__u8 *buffer, struct ve_struct *ve,
@@ -147,8 +149,15 @@ static struct cn_msg *cn_msg_fill(__u8 *buffer, struct ve_struct *ve,
 
 static int proc_event_num_listeners(struct ve_struct *ve)
 {
-	if (ve->cn)
-		return atomic_read(&ve->cn->proc_event_num_listeners);
+	/*
+	 * Callers not under rcu_read_lock() are pinned by a live task
+	 * of this VE (or run on ve0 whose connector state lives as long
+	 * as the connector itself).
+	 */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
+
+	if (cn)
+		return atomic_read(&cn->proc_event_num_listeners);
 	return 0;
 }
 
@@ -414,6 +423,8 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
 	enum proc_cn_event ev_type = 0;
 	int err = 0, initial = 0;
 	struct sock *sk = NULL;
+	/* current is a live task of this VE, it cannot be stopped under us */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 
 	/* 
 	 * Events are reported with respect to the initial pid
@@ -467,11 +478,11 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
 	switch (mc_op) {
 	case PROC_CN_MCAST_LISTEN:
 		if (initial || (prev_mc_op != PROC_CN_MCAST_LISTEN))
-			atomic_inc(&ve->cn->proc_event_num_listeners);
+			atomic_inc(&cn->proc_event_num_listeners);
 		break;
 	case PROC_CN_MCAST_IGNORE:
 		if (!initial && (prev_mc_op != PROC_CN_MCAST_IGNORE))
-			atomic_dec(&ve->cn->proc_event_num_listeners);
+			atomic_dec(&cn->proc_event_num_listeners);
 		((struct proc_input *)(sk->sk_user_data))->event_type =
 			PROC_EVENT_NONE;
 		break;
@@ -488,13 +499,18 @@ int cn_proc_init_ve(struct ve_struct *ve)
 {
 	int err, cpu;
 	struct local_event *le_ptr;
+	struct cn_private *cn;
 
-	ve->cn->local_event = alloc_percpu(struct local_event);
-	if (!ve->cn->local_event)
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+
+	cn->local_event = alloc_percpu(struct local_event);
+	if (!cn->local_event)
 		return -ENOMEM;
 
 	for_each_possible_cpu(cpu) {
-		le_ptr = per_cpu_ptr(ve->cn->local_event, cpu);
+		le_ptr = per_cpu_ptr(cn->local_event, cpu);
 		local_lock_init(&le_ptr->lock);
 	}
 
@@ -503,15 +519,21 @@ int cn_proc_init_ve(struct ve_struct *ve)
 				  &cn_proc_mcast_ctl);
 	if (err) {
 		pr_warn("VE#%d: cn_proc failed to register\n", ve->veid);
-		free_percpu(ve->cn->local_event);
+		free_percpu(cn->local_event);
 		return err;
 	}
-	atomic_set(&ve->cn->proc_event_num_listeners, 0);
+	atomic_set(&cn->proc_event_num_listeners, 0);
 	return 0;
 }
 
 void cn_proc_fini_ve(struct ve_struct *ve)
 {
+	struct cn_private *cn;
+
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+
 	cn_del_callback_ve(ve, &cn_proc_event_id);
-	free_percpu(ve->cn->local_event);
+	free_percpu(cn->local_event);
 }
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 39a697803d0f1..bf4a83f3f8703 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -29,7 +29,13 @@ MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_CONNECTOR);
 
 static struct cn_dev *get_cdev(struct ve_struct *ve)
 {
-	return &ve->cn->cdev;
+	/*
+	 * Callers not under rcu_read_lock() are pinned by a live task
+	 * of this VE or run from the VE start/stop hooks.
+	 */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
+
+	return cn ? &cn->cdev : NULL;
 }
 
 /*
@@ -230,12 +236,13 @@ int cn_add_callback_ve(struct ve_struct *ve,
 		       void (*callback)(struct cn_msg *,
 					struct netlink_skb_parms *))
 {
-	struct cn_dev *dev = get_cdev(ve);
+	/* See the comment in get_cdev() */
+	struct cn_private *cn = rcu_dereference_check(ve->cn, 1);
 
-	if (!ve->cn->cn_already_initialized)
+	if (!cn || !cn->cn_already_initialized)
 		return -EAGAIN;
 
-	return cn_queue_add_callback(dev->cbdev, name, id, callback);
+	return cn_queue_add_callback(cn->cdev.cbdev, name, id, callback);
 }
 
 /*
@@ -297,6 +304,7 @@ static int __maybe_unused cn_proc_show(struct seq_file *m, void *v)
 static int cn_init_ve(void *data)
 {
 	struct ve_struct *ve = data;
+	struct cn_private *cn;
 	struct cn_dev *dev;
 	struct netlink_kernel_cfg cfg = {
 		.groups	= CN_NETLINK_USERS + 0xf,
@@ -308,11 +316,12 @@ static int cn_init_ve(void *data)
 	struct net *net;
 	int err;
 
-	ve->cn = kzalloc(sizeof(*ve->cn), GFP_KERNEL);
-	if (!ve->cn)
+	cn = kzalloc(sizeof(*cn), GFP_KERNEL);
+	if (!cn)
 		return -ENOMEM;
+	rcu_assign_pointer(ve->cn, cn);
 
-	dev = &ve->cn->cdev;
+	dev = &cn->cdev;
 
 	/*
 	 * This is a hook, hooks are called under a single lock, so ve_nsproxy will
@@ -331,7 +340,7 @@ static int cn_init_ve(void *data)
 		goto netlink_release;
 	}
 
-	ve->cn->cn_already_initialized = 1;
+	cn->cn_already_initialized = 1;
 
 	if (!proc_ve_create_single("connector", S_IRUGO, net->proc_net,
 				cn_proc_show)) {
@@ -353,18 +362,24 @@ static int cn_init_ve(void *data)
 netlink_release:
 	netlink_kernel_release(dev->nls);
 free_cn:
-	kfree(ve->cn);
-	ve->cn = NULL;
+	RCU_INIT_POINTER(ve->cn, NULL);
+	kfree(cn);
 	goto net_unlock;
 }
 
 static void cn_fini_ve(void *data)
 {
 	struct ve_struct *ve = data;
-	struct cn_dev *dev = get_cdev(ve);
+	struct cn_private *cn;
+	struct cn_dev *dev;
 	struct net *net;
 
-	ve->cn->cn_already_initialized = 0;
+	cn = rcu_dereference_protected(ve->cn,
+				       lockdep_is_held(&ve->op_sem) ||
+				       ve_is_super(ve));
+	dev = &cn->cdev;
+
+	cn->cn_already_initialized = 0;
 
 	cn_proc_fini_ve(ve);
 
@@ -379,8 +394,8 @@ static void cn_fini_ve(void *data)
 	cn_queue_free_dev(dev->cbdev);
 	netlink_kernel_release(dev->nls);
 
-	kfree(ve->cn);
-	ve->cn = NULL;
+	RCU_INIT_POINTER(ve->cn, NULL);
+	kfree(cn);
 }
 
 #ifdef CONFIG_VE
diff --git a/include/linux/ve.h b/include/linux/ve.h
index 46e67d00cf783..2dc93e42784b2 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -102,7 +102,7 @@ struct ve_struct {
 	char			core_pattern[CORENAME_MAX_SIZE];
 #endif
 #ifdef CONFIG_CONNECTOR
-	struct cn_private	*cn;
+	struct cn_private __rcu	*cn;
 #endif
 
 	struct kthread_worker	*kthreadd_worker;
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Devel] [PATCH RHEL10 COMMIT] connector: free the per-VE connector state after an RCU grace period
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 2/4] connector: free the per-VE connector state after an RCU grace period Vasileios Almpanis
@ 2026-08-26 13:09   ` Konstantin Khorenko
  0 siblings, 0 replies; 9+ messages in thread
From: Konstantin Khorenko @ 2026-08-26 13:09 UTC (permalink / raw)
  To: Vasileios Almpanis; +Cc: OpenVZ devel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Devel] [PATCH RHEL10 COMMIT] connector: deliver per-VE proc events under an RCU read lock
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 3/4] connector: deliver per-VE proc events under an RCU read lock Vasileios Almpanis
@ 2026-08-26 13:09   ` Konstantin Khorenko
  0 siblings, 0 replies; 9+ messages in thread
From: Konstantin Khorenko @ 2026-08-26 13:09 UTC (permalink / raw)
  To: Vasileios Almpanis; +Cc: OpenVZ devel

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 d502d5ac74069447eecc819e568b1f626a554e46
Author: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
Date:   Tue Aug 25 16:19:53 2026 +0000

    connector: deliver per-VE proc events under an RCU read lock
    
    proc_event_connector_ve() dereferences ve->cn several times and
    fill_exit_event() dereferences ve->ve_nsproxy assuming the VE cannot
    stop in the middle of the delivery. This holds while the reported task
    is alive in the VE: a live task keeps the VE pid namespace busy, so
    zap_pid_ns_processes() -> ve_exit_ns() cannot start.
    
    The next patch makes proc_exit_connector() deliver the exit event with
    a VE reference pinned before exit_notify(). Once the task is reaped,
    its pid no longer keeps the pid namespace busy: the container init may
    be woken up by free_pid() from release_task(), finish
    zap_pid_ns_processes() and run ve_exit_ns() while the exit event is
    still being delivered:
    
        cpu0: exiting task                cpu1: container init
        do_exit()
          exit_notify()
            release_task()
              free_pid()  ------------->  wakes zap_pid_ns_processes()
          proc_exit_connector()           ve_exit_ns()
            proc_event_connector_ve()       cn_fini_ve()      /* ve->cn */
              fill_exit_event()             ve_drop_context() /* ve_nsproxy */
                ve->ve_nsproxy->...
    
    Deliver the event under rcu_read_lock() and recheck the pointers: the
    previous patch guarantees everything reachable from ve->cn stays alive
    for the whole read-side critical section once observed, and
    ve_drop_context() already waits for a grace period before dropping
    ve_nsproxy. Bail out if the VE is being stopped: its listeners are
    dead anyway, there is nobody to deliver to.
    
    The whole delivery path runs with GFP_NOWAIT and never sleeps, so it
    is legal inside an RCU read-side critical section.
    
    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   | 36 +++++++++++++++++++++++++++++++++---
 drivers/connector/connector.c |  4 ++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 6095c7def7cea..6084308085489 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -89,6 +89,10 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	struct local_event *le_ptr;
 	__u32 filter_data[2];
 
+	/* The VE is being stopped, so are its listeners: nothing to do */
+	if (!cn)
+		return;
+
 	/*
 	 * The following hack with local_event->lock address works only
 	 * till the "lock" is the first field in the local_event struct,
@@ -172,15 +176,27 @@ static void proc_event_connector_ve(struct task_struct *task,
 	struct cn_msg *msg;
 	__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
 
+	/*
+	 * The exit event may be delivered when the reported task no
+	 * longer pins the VE (see proc_exit_connector()), so the VE may
+	 * be stopping concurrently. cn_proc_fini_ve() waits for an RCU
+	 * grace period before the connector state is freed, take the RCU
+	 * read lock to make the state observed here stay valid for the
+	 * whole delivery. The path below never sleeps (GFP_NOWAIT).
+	 */
+	rcu_read_lock();
+
 	if (proc_event_num_listeners(ve) < 1)
-		return;
+		goto out_unlock;
 
 	msg = cn_msg_fill(buffer, ve, task, what, cookie, fill_event);
 	if (!msg)
-		return;
+		goto out_unlock;
 
 	/*  If cn_netlink_send() failed, the data is not sent */
 	send_msg_ve(ve, msg);
+out_unlock:
+	rcu_read_unlock();
 }
 
 static void proc_event_connector(struct task_struct *task,
@@ -350,9 +366,23 @@ void proc_coredump_connector(struct task_struct *task)
 static bool fill_exit_event(struct proc_event *ev, struct ve_struct *ve,
 			    struct task_struct *task, long cookie_pids)
 {
-	struct pid_namespace *pid_ns = ve->ve_nsproxy->pid_ns_for_children;
+	struct pid_namespace *pid_ns;
 	struct task_struct *parent;
 	struct pids *pids = (struct pids *)cookie_pids;
+	struct nsproxy *nsproxy;
+
+	/*
+	 * Unlike all other events, the exit event may be delivered after
+	 * the task was reaped, when nothing keeps the VE pid namespace
+	 * busy anymore and the VE may be stopping concurrently.
+	 * ve_drop_context() clears ve_nsproxy and waits for an RCU grace
+	 * period before dropping it; we are called under rcu_read_lock().
+	 * The VE is dead, so are its listeners: skip the event.
+	 */
+	nsproxy = rcu_dereference(ve->ve_nsproxy);
+	if (!nsproxy)
+		return false;
+	pid_ns = nsproxy->pid_ns_for_children;
 
 	ev->event_data.exit.process_pid = pid_nr_ns(pids->pid, pid_ns);
 	ev->event_data.exit.process_tgid = pid_nr_ns(pids->tgid, pid_ns);
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 05c281bb321be..5597801c4af28 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -79,6 +79,10 @@ int cn_netlink_send_mult_ve(struct ve_struct *ve, struct cn_msg *msg, u16 len,
 	u32 group = 0;
 	int found = 0;
 
+	/* The VE is being stopped, see proc_event_connector_ve() */
+	if (!dev)
+		return -ENODEV;
+
 	if (portid || __group) {
 		group = __group;
 	} else {
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Devel] [PATCH RHEL10 COMMIT] proc connector: pin task VE for the exit event notification
  2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 4/4] proc connector: pin task VE for the exit event notification Vasileios Almpanis
@ 2026-08-26 13:09   ` Konstantin Khorenko
  0 siblings, 0 replies; 9+ messages in thread
From: Konstantin Khorenko @ 2026-08-26 13:09 UTC (permalink / raw)
  To: Vasileios Almpanis; +Cc: OpenVZ devel

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 5d93a033f4ceafc086a81687ac873c236e2f2d9e
Author: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
Date:   Tue Aug 25 16:19:54 2026 +0000

    proc connector: pin task VE for the exit event notification
    
    The exit event is reported after exit_notify(), so the parent might have
    been woken up and reaped the exiting task via wait() -> release_task()
    -> exit_ve_namespace(), which resets tsk->task_ve to ve0. If that wins
    the race against the exiting task then the event will only be delivered
    to the host VE listeners and the in-VE listeners will be skipped. For a
    task inside a container this means the listener never receives an exit
    event.
    
    The issue is caught by the LTP suite_kernel_misc.exec.cn_pec_sh test:
    pec_listener terminates upon receiving the exit event of the pid given
    via -p. When the lost exit event is a child's one, the test fails:
    
      cn_pec 3 TFAIL: Event was not detected by the event listener:
                      exit pid: 58388 exit_code: 0 exit_signal: 17
    
    and when it is the event generator's own exit event, the listener
    polls the netlink socket forever and the test hangs until the LTP
    timeout kills it (~8h on coverage kernels):
    
    22:51:37 cn_pec 2 TINFO: Testing exec event (nevents=10)
    07:11:37 Test timed out, sending SIGTERM!
    
    The race was captured using kprobes on the connector send path:
      p:cnp/pexit proc_exit_connector task=$arg1:x64
      p:cnp/vexit exit_ve_namespace task=$arg1:x64
      p:cnp/pevcve proc_event_connector_ve what=$arg3:u32
      r:cnp/cnsend cn_netlink_send_mult_ve ret=$retval:s64
    
    A normal exit looks like:
      pexit -> pevcve(ve) -> cnsend ret=0 -> pevcve(ve0)
    
    The lost event (task 0xffff89a506d73980 is the exiting child, reaped by
    its parent pid 465820 in between):
    
     465831 [001] pexit: (proc_exit_connector) task=0xffff89a506d73980
     465820 [002] vexit: (exit_ve_namespace) task=0xffff89a506d73980
     465831 [001] pevcve: (proc_event_connector_ve) what=2147483648
    
    Only one proc_event_connector_ve() call fires (ve0, no listeners) and
    cn_netlink_send_mult_ve() is never reached: the event is dropped.
    
    This is the same race window that commit c565cc211694 ("proc
    connector: report proper pid/tgid of an exited process") closed for
    the task pid/tgid, but nothing pins the VE.
    
    Reproducer (fails within ~50 iterations in a CT on a coverage kernel):
    
      cd /opt/ltp/testcases/bin/
      export PATH=$PATH:/opt/ltp/testcases/bin
      for i in $(seq 1 1024); do cn_pec.sh >/dev/null 2>&1 || break; done
    
    Solve this the same way. Pin the VE in do_exit() before exit_notify()
    is called and use it in proc_exit_connector() instead of re-reading
    task->task_ve.
    
    https://virtuozzo.atlassian.net/browse/VSTOR-140421
    Fixes: 95fa2f096b72 ("ve: Introduce VE namespace")
    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 | 10 ++++++++--
 include/linux/cn_proc.h     |  8 ++++++--
 kernel/exit.c               |  5 ++++-
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 6084308085489..8d8c538cc3cd4 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -401,9 +401,15 @@ static bool fill_exit_event(struct proc_event *ev, struct ve_struct *ve,
 	return true;
 }
 
-void proc_exit_connector(struct task_struct *task, struct pids *pids)
+void proc_exit_connector(struct task_struct *task, struct pids *pids,
+			 struct ve_struct *ve)
 {
-	proc_event_connector(task, PROC_EVENT_EXIT, (long)pids, fill_exit_event);
+	if (!ve_is_super(ve))
+		proc_event_connector_ve(task, ve, PROC_EVENT_EXIT, (long)pids,
+					fill_exit_event);
+
+	proc_event_connector_ve(task, get_ve0(), PROC_EVENT_EXIT, (long)pids,
+				fill_exit_event);
 }
 
 /*
diff --git a/include/linux/cn_proc.h b/include/linux/cn_proc.h
index 9701c13d82dfc..aa606192b5c2e 100644
--- a/include/linux/cn_proc.h
+++ b/include/linux/cn_proc.h
@@ -19,6 +19,8 @@
 
 #include <uapi/linux/cn_proc.h>
 
+struct ve_struct;
+
 /*
  * The struct is used solely for pinning task pids for proc connector
  * notification on process exit.
@@ -36,7 +38,8 @@ void proc_sid_connector(struct task_struct *task);
 void proc_ptrace_connector(struct task_struct *task, int which_id);
 void proc_comm_connector(struct task_struct *task);
 void proc_coredump_connector(struct task_struct *task);
-void proc_exit_connector(struct task_struct *task, struct pids *pids);
+void proc_exit_connector(struct task_struct *task, struct pids *pids,
+			 struct ve_struct *ve);
 #else
 static inline void proc_fork_connector(struct task_struct *task)
 {}
@@ -61,7 +64,8 @@ static inline void proc_ptrace_connector(struct task_struct *task,
 static inline void proc_coredump_connector(struct task_struct *task)
 {}
 
-static inline void proc_exit_connector(struct task_struct *task, struct pids *pids)
+static inline void proc_exit_connector(struct task_struct *task, struct pids *pids,
+				       struct ve_struct *ve)
 {}
 #endif	/* CONFIG_PROC_EVENTS */
 #endif	/* CN_PROC_H */
diff --git a/kernel/exit.c b/kernel/exit.c
index 448a734270a7f..94d9bddae2b86 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -936,6 +936,7 @@ void __noreturn do_exit(long code)
 	struct task_struct *tsk = current;
 	int group_dead;
 	struct pids pids;
+	struct ve_struct *ve;
 
 	WARN_ON(irqs_disabled());
 
@@ -1021,8 +1022,10 @@ void __noreturn do_exit(long code)
 	exit_tasks_rcu_start();
 	pids.pid = get_pid(task_pid(tsk));
 	pids.tgid = get_pid(task_tgid(tsk));
+	ve = get_task_ve(tsk);
 	exit_notify(tsk, group_dead);
-	proc_exit_connector(tsk, &pids);
+	proc_exit_connector(tsk, &pids, ve);
+	put_ve(ve);
 	put_pid(pids.tgid);
 	put_pid(pids.pid);
 	mpol_put_task_policy(tsk);
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-08-26 13:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-25 16:19 [Devel] [PATCH VZ10 v3 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 1/4] connector: annotate ve->cn with __rcu Vasileios Almpanis
2026-08-26 13:09   ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko
2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 2/4] connector: free the per-VE connector state after an RCU grace period Vasileios Almpanis
2026-08-26 13:09   ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko
2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 3/4] connector: deliver per-VE proc events under an RCU read lock Vasileios Almpanis
2026-08-26 13:09   ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko
2026-08-25 16:19 ` [Devel] [PATCH VZ10 v3 4/4] proc connector: pin task VE for the exit event notification Vasileios Almpanis
2026-08-26 13:09   ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox