* [Devel] [PATCH VZ10 v2 1/4] connector: annotate ve->cn with __rcu
2026-08-18 15:10 [Devel] [PATCH VZ10 v2 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
@ 2026-08-18 15:10 ` Vasileios Almpanis
2026-08-25 11:58 ` Konstantin Khorenko
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 2/4] connector: free the per-VE connector state after an RCU grace period Vasileios Almpanis
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Vasileios Almpanis @ 2026-08-18 15:10 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 cba827260d07..3334bd1e9517 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -94,7 +94,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] 8+ messages in thread* Re: [Devel] [PATCH VZ10 v2 1/4] connector: annotate ve->cn with __rcu
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 1/4] connector: annotate ve->cn with __rcu Vasileios Almpanis
@ 2026-08-25 11:58 ` Konstantin Khorenko
0 siblings, 0 replies; 8+ messages in thread
From: Konstantin Khorenko @ 2026-08-25 11:58 UTC (permalink / raw)
On 8/18/26 17:10, Vasileios Almpanis wrote:
> 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(-)
>
...
> @@ -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);
The attribution logic:
- The error path itself (RCU_INIT_POINTER(ve->cn, NULL); kfree(cn) in cn_init_ve()) appeared in patch 1, where it
faithfully preserves the base behavior (ve->cn = NULL; kfree) - the "no functional change" claim holds.
- Before patch 4, the only possible reader of a container VE's ve->cn is a live task of that VE. While cn_init_ve() runs
(VE start, under op_sem) there are no tasks in the VE yet - no reader can exist, so the immediate kfree is safe.
- Patch 4 creates a new kind of reader: the exit event delivery with a pinned ve_struct, which can run after the VE has
fully stopped - and therefore also concurrently with its restart (it is the very same ve_struct, the cgroup stays
alive). Bisect-wise the crash only becomes reachable starting with patch 4 - hence the question sits in its
review-inline.
The scenario:
CPU0: exiting task (old CT incarnation) CPU1: vzctl start (restart of the same ve)
========================================= ========================================
do_exit()
ve = get_task_ve(tsk) /* pins ve */
exit_notify() /* reaped, pid
freed */
<-- preempted; the CT manages to stop
completely: cn_fini_ve() done,
ve->cn == NULL -->
ve_start_container() /* op_sem */
cn_init_ve()
cn = kzalloc(...)
rcu_assign_pointer(ve->cn, cn) <-- published
proc_exit_connector(tsk, &pids, ve)
proc_event_connector_ve(task, ve, ...)
rcu_read_lock()
proc_event_num_listeners(ve)
cn = rcu_dereference_check(ve->cn, 1)
/* sees the FRESH cn from CPU1 */
netlink_kernel_create() -> fail
free_cn:
RCU_INIT_POINTER(ve->cn, NULL)
kfree(cn) <-- NO grace period
atomic_read(&cn->proc_event_num_listeners)
*** use-after-free ***
The reader dutifully sits under rcu_read_lock(), but unlike cn_proc_fini_ve() the error path does not wait for a grace
period - patch 2's RCU contract ("everything reachable from ve->cn is freed only after a grace period") was not extended
to this freeing site.
That said, the right place to fix it is patch 2 rather than patch 4 - patch 2 owns the contract, and with the fix in place
patch 4 lands on a fully safe base. The minimal variant, symmetric to cn_proc_fini_ve():
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ static int cn_init_ve(void *data)
free_cn:
RCU_INIT_POINTER(ve->cn, NULL);
+ /*
+ * The pointer was published, so a pinned-VE exit event delivery
+ * may already be looking at it under rcu_read_lock(): wait for
+ * the readers before freeing, same as cn_proc_fini_ve() does.
+ */
+ synchronize_rcu();
kfree(cn);
goto net_unlock;
The context is sleepable (GFP_KERNEL right nearby), and sleeping under op_sem is fine - ve_stop_ns()/cn_proc_fini_ve()
already do exactly that.
The architecturally cleaner alternative is to publish ve->cn only after the initialization fully succeeds (then a reader
can never see a half-constructed state and the error path stays a plain kfree). But that is a bigger refactor:
cn_proc_init_ve() and cn_add_callback_ve() dereference ve->cn themselves, so they would need to take cn as an argument.
For this series the one-line synchronize_rcu() in the error path looks more appropriate.
> 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 cba827260d07..3334bd1e9517 100644
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h
> @@ -94,7 +94,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;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Devel] [PATCH VZ10 v2 2/4] connector: free the per-VE connector state after an RCU grace period
2026-08-18 15:10 [Devel] [PATCH VZ10 v2 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 1/4] connector: annotate ve->cn with __rcu Vasileios Almpanis
@ 2026-08-18 15:10 ` Vasileios Almpanis
2026-08-25 11:47 ` Konstantin Khorenko
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 3/4] connector: deliver per-VE proc events under an RCU read lock Vasileios Almpanis
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 4/4] proc connector: pin task VE for the exit event notification Vasileios Almpanis
3 siblings, 1 reply; 8+ messages in thread
From: Vasileios Almpanis @ 2026-08-18 15:10 UTC (permalink / raw)
cn_fini_ve() tears down everything the proc event delivery path uses
and 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. This will make the teardown able
to 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. 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().
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 | 2 +-
2 files changed, 12 insertions(+), 1 deletion(-)
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..6483e15b888e 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -394,7 +394,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] 8+ messages in thread
* Re: [Devel] [PATCH VZ10 v2 2/4] connector: free the per-VE connector state after an RCU grace period
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 2/4] connector: free the per-VE connector state after an RCU grace period Vasileios Almpanis
@ 2026-08-25 11:47 ` Konstantin Khorenko
2026-08-25 12:43 ` Vasileios Almpanis
0 siblings, 1 reply; 8+ messages in thread
From: Konstantin Khorenko @ 2026-08-25 11:47 UTC (permalink / raw)
On 8/18/26 17:10, Vasileios Almpanis wrote:
> cn_fini_ve() tears down everything the proc event delivery path uses
> and 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. This will make the teardown able
> to 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. 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().
>
> 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 | 2 +-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> 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();
> +
[Severity: Medium]
Can this crash the netlink receive path while the VE is stopping?
Clearing ve->cn now happens long before the kernel netlink socket is
released in cn_fini_ve(): the window covers this synchronize_rcu(),
free_percpu(), remove_proc_entry() and cn_queue_free_dev(). Within
that window dev->nls is still alive and accepts messages, and a send
to it ends up in:
drivers/connector/connector.c:cn_call_callback() {
...
struct cn_dev *dev = get_cdev(sock_net(skb->sk)->owner_ve);
...
spin_lock_bh(&dev->cbdev->queue_lock);
...
}
Since the previous patch get_cdev() returns NULL once ve->cn is
cleared, so this dereferences a NULL dev.
All in-VE tasks are dead at this point, but the VE netns can still be
entered from the host (nsenter, criu and vzctl network tooling), and a
NETLINK_CONNECTOR socket created there delivers straight into
cn_rx_skb() -> cn_call_callback() in the sender's context.
Before this patch the pointer was cleared only after
netlink_kernel_release(), which quiesces the input path first, so this
window did not exist. The next patch adds a NULL check to the send
side (cn_netlink_send_mult_ve()) but not to the receive side.
Would a NULL check in cn_call_callback(), or clearing ve->cn only
after netlink_kernel_release() in cn_fini_ve() while keeping the
grace period before the frees, close this?
=============================================================
? Scenario 1 - NULL deref (fetch after the clear)
Precondition: the container is stopping - all CT tasks are dead, the container init is finishing zap_pid_ns_processes().
On the host there is a process (criu / vzctl tooling / nsenter) that has opened a NETLINK_CONNECTOR socket in this CT's
netns beforehand (the socket keeps the netns alive).
CPU0: container init (last CT task) CPU1: host process with a socket in the CT netns
========================================= ============================================
do_exit()
exit_notify()
zap_pid_ns_processes() /* pid_ns is empty */
ve_exit_ns()
down_write(&ve->op_sem)
ve_hook_iterate_fini()
cn_fini_ve()
cn = rcu_dereference_protected(ve->cn)
cn->cn_already_initialized = 0
cn_proc_fini_ve()
cn_del_callback_ve()
RCU_INIT_POINTER(ve->cn, NULL) <--- pointer is hidden
synchronize_rcu() <--- sleeps for milliseconds,
... dev->nls is still ALIVE
... sendmsg(nl_sock, msg)
... netlink_sendmsg()
... netlink_unicast()
... netlink_unicast_kernel()
... nlk->netlink_rcv = cn_rx_skb()
... cn_call_callback()
... dev = get_cdev(net->owner_ve)
... rcu_dereference_check(ve->cn, 1)
... /* ve->cn == NULL */
... return NULL; <--- patch 1
... spin_lock_bh(&dev->cbdev->queue_lock)
... /* dev == NULL */
... *** oops: NULL + offsetof(cbdev) ***
free_percpu(cn->local_event)
remove_proc_entry("connector", ...)
cn_queue_free_dev(dev->cbdev)
netlink_kernel_release(dev->nls) <--- only HERE does the input path die,
kfree(cn) but it is too late
The key point: the window between RCU_INIT_POINTER(ve->cn, NULL) and netlink_kernel_release() is now wide (it contains a
whole synchronize_rcu()), the kernel socket keeps accepting messages all that time, and cn_call_callback() on the receive
side has no NULL check. Before patch 2 the order was reversed: netlink_kernel_release() quiesced the input path first, and
only then ve->cn = NULL - within that window CPU1 simply could not reach cn_call_callback().
? Scenario 2 - UAF (fetch before the clear, use after the free)
Same setup, but CPU1 fetched the pointer before the clear and got preempted. synchronize_rcu() does not wait for it -
cn_call_callback() reads ve->cn without rcu_read_lock():
CPU0: container init CPU1: host-side sender
========================================= ============================================
cn_rx_skb()
cn_call_callback()
dev = get_cdev(...) /* cn != NULL, ok */
<-- preempted -->
cn_proc_fini_ve()
RCU_INIT_POINTER(ve->cn, NULL)
synchronize_rcu() /* CPU1 is not in an RCU
section - it will NOT
be waited for */
free_percpu(cn->local_event)
cn_queue_free_dev(dev->cbdev) <--- cbdev is freed
netlink_kernel_release(dev->nls)
kfree(cn) <--- cn is freed
<-- resumes -->
spin_lock_bh(&dev->cbdev->queue_lock)
*** use-after-free: dev = &cn->cdev,
cn and cbdev already kfree()d ***
Scenario 2 is exactly why a bare if (!dev) return in cn_call_callback() is not enough: it only fixes scenario 1. The
proposed patch takes rcu_read_lock() around the fetch plus the queue walk - then in scenario 2 CPU1 is inside an RCU
section, synchronize_rcu() on CPU0 has to wait for it and all the frees move past the end of the section; and in scenario
1 the NULL check kicks in.
=============================================================
suggested fix:
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -154,7 +154,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;
@@ -164,6 +164,19 @@ static int cn_call_callback(struct sk_buff *skb)
if (nlh->nlmsg_len < NLMSG_HDRLEN + sizeof(struct cn_msg) + msg->len)
return -EINVAL;
+ /*
+ * The kernel socket outlives ve->cn on VE stop: cn_proc_fini_ve()
+ * clears the pointer and waits for a grace period long before
+ * cn_fini_ve() releases the socket, so a message sent from the
+ * host into the dying VE netns can still get here. Fetch ve->cn
+ * and walk the callback queue under rcu_read_lock(): once the
+ * pointer is observed, the grace period in cn_proc_fini_ve()
+ * keeps the callback device alive until we are done with it.
+ */
+ 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)) {
@@ -173,6 +186,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);
> free_percpu(cn->local_event);
> }
> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
> index bf4a83f3f870..6483e15b888e 100644
> --- a/drivers/connector/connector.c
> +++ b/drivers/connector/connector.c
> @@ -394,7 +394,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);
> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Devel] [PATCH VZ10 v2 2/4] connector: free the per-VE connector state after an RCU grace period
2026-08-25 11:47 ` Konstantin Khorenko
@ 2026-08-25 12:43 ` Vasileios Almpanis
0 siblings, 0 replies; 8+ messages in thread
From: Vasileios Almpanis @ 2026-08-25 12:43 UTC (permalink / raw)
On 8/25/26 1:47 PM, Konstantin Khorenko wrote:
> On 8/18/26 17:10, Vasileios Almpanis wrote:
>> cn_fini_ve() tears down everything the proc event delivery path uses
>> and 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. This will make the teardown able
>> to 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. 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().
>>
>> 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 | 2 +-
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> 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();
>> +
> [Severity: Medium]
> Can this crash the netlink receive path while the VE is stopping?
>
> Clearing ve->cn now happens long before the kernel netlink socket is
> released in cn_fini_ve(): the window covers this synchronize_rcu(),
> free_percpu(), remove_proc_entry() and cn_queue_free_dev(). Within
> that window dev->nls is still alive and accepts messages, and a send
> to it ends up in:
>
> drivers/connector/connector.c:cn_call_callback() {
> ...
> struct cn_dev *dev = get_cdev(sock_net(skb->sk)->owner_ve);
> ...
> spin_lock_bh(&dev->cbdev->queue_lock);
> ...
> }
>
> Since the previous patch get_cdev() returns NULL once ve->cn is
> cleared, so this dereferences a NULL dev.
>
> All in-VE tasks are dead at this point, but the VE netns can still be
> entered from the host (nsenter, criu and vzctl network tooling), and a
> NETLINK_CONNECTOR socket created there delivers straight into
> cn_rx_skb() -> cn_call_callback() in the sender's context.
>
> Before this patch the pointer was cleared only after
> netlink_kernel_release(), which quiesces the input path first, so this
> window did not exist. The next patch adds a NULL check to the send
> side (cn_netlink_send_mult_ve()) but not to the receive side.
>
> Would a NULL check in cn_call_callback(), or clearing ve->cn only
> after netlink_kernel_release() in cn_fini_ve() while keeping the
> grace period before the frees, close this?
I think a NULL check on the receive side is the better option. The send
path uses dev->nls under RCU, so the socket must outlive the grace period,
which means the clear must precede the release. I'll add it in v3
>
> =============================================================
> ? Scenario 1 - NULL deref (fetch after the clear)
>
> Precondition: the container is stopping - all CT tasks are dead, the container init is finishing zap_pid_ns_processes().
> On the host there is a process (criu / vzctl tooling / nsenter) that has opened a NETLINK_CONNECTOR socket in this CT's
> netns beforehand (the socket keeps the netns alive).
>
> CPU0: container init (last CT task) CPU1: host process with a socket in the CT netns
> ========================================= ============================================
> do_exit()
> exit_notify()
> zap_pid_ns_processes() /* pid_ns is empty */
> ve_exit_ns()
> down_write(&ve->op_sem)
> ve_hook_iterate_fini()
> cn_fini_ve()
> cn = rcu_dereference_protected(ve->cn)
> cn->cn_already_initialized = 0
> cn_proc_fini_ve()
> cn_del_callback_ve()
> RCU_INIT_POINTER(ve->cn, NULL) <--- pointer is hidden
> synchronize_rcu() <--- sleeps for milliseconds,
> ... dev->nls is still ALIVE
> ... sendmsg(nl_sock, msg)
> ... netlink_sendmsg()
> ... netlink_unicast()
> ... netlink_unicast_kernel()
> ... nlk->netlink_rcv = cn_rx_skb()
> ... cn_call_callback()
> ... dev = get_cdev(net->owner_ve)
> ... rcu_dereference_check(ve->cn, 1)
> ... /* ve->cn == NULL */
> ... return NULL; <--- patch 1
> ... spin_lock_bh(&dev->cbdev->queue_lock)
> ... /* dev == NULL */
> ... *** oops: NULL + offsetof(cbdev) ***
> free_percpu(cn->local_event)
> remove_proc_entry("connector", ...)
> cn_queue_free_dev(dev->cbdev)
> netlink_kernel_release(dev->nls) <--- only HERE does the input path die,
> kfree(cn) but it is too late
>
> The key point: the window between RCU_INIT_POINTER(ve->cn, NULL) and netlink_kernel_release() is now wide (it contains a
> whole synchronize_rcu()), the kernel socket keeps accepting messages all that time, and cn_call_callback() on the receive
> side has no NULL check. Before patch 2 the order was reversed: netlink_kernel_release() quiesced the input path first, and
> only then ve->cn = NULL - within that window CPU1 simply could not reach cn_call_callback().
>
>
> ? Scenario 2 - UAF (fetch before the clear, use after the free)
>
> Same setup, but CPU1 fetched the pointer before the clear and got preempted. synchronize_rcu() does not wait for it -
> cn_call_callback() reads ve->cn without rcu_read_lock():
>
> CPU0: container init CPU1: host-side sender
> ========================================= ============================================
> cn_rx_skb()
> cn_call_callback()
> dev = get_cdev(...) /* cn != NULL, ok */
> <-- preempted -->
> cn_proc_fini_ve()
> RCU_INIT_POINTER(ve->cn, NULL)
> synchronize_rcu() /* CPU1 is not in an RCU
> section - it will NOT
> be waited for */
> free_percpu(cn->local_event)
> cn_queue_free_dev(dev->cbdev) <--- cbdev is freed
> netlink_kernel_release(dev->nls)
> kfree(cn) <--- cn is freed
> <-- resumes -->
> spin_lock_bh(&dev->cbdev->queue_lock)
> *** use-after-free: dev = &cn->cdev,
> cn and cbdev already kfree()d ***
>
> Scenario 2 is exactly why a bare if (!dev) return in cn_call_callback() is not enough: it only fixes scenario 1. The
> proposed patch takes rcu_read_lock() around the fetch plus the queue walk - then in scenario 2 CPU1 is inside an RCU
> section, synchronize_rcu() on CPU0 has to wait for it and all the frees move past the end of the section; and in scenario
> 1 the NULL check kicks in.
> =============================================================
>
>
> suggested fix:
>
> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
> --- a/drivers/connector/connector.c
> +++ b/drivers/connector/connector.c
> @@ -154,7 +154,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;
> @@ -164,6 +164,19 @@ static int cn_call_callback(struct sk_buff *skb)
> if (nlh->nlmsg_len < NLMSG_HDRLEN + sizeof(struct cn_msg) + msg->len)
> return -EINVAL;
>
> + /*
> + * The kernel socket outlives ve->cn on VE stop: cn_proc_fini_ve()
> + * clears the pointer and waits for a grace period long before
> + * cn_fini_ve() releases the socket, so a message sent from the
> + * host into the dying VE netns can still get here. Fetch ve->cn
> + * and walk the callback queue under rcu_read_lock(): once the
> + * pointer is observed, the grace period in cn_proc_fini_ve()
> + * keeps the callback device alive until we are done with it.
> + */
> + 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)) {
> @@ -173,6 +186,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);
>
>> free_percpu(cn->local_event);
>> }
>> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
>> index bf4a83f3f870..6483e15b888e 100644
>> --- a/drivers/connector/connector.c
>> +++ b/drivers/connector/connector.c
>> @@ -394,7 +394,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);
>> }
>>
>>
--
Best regards, Vasileios Almpanis
Software Developer, Virtuozzo.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Devel] [PATCH VZ10 v2 3/4] connector: deliver per-VE proc events under an RCU read lock
2026-08-18 15:10 [Devel] [PATCH VZ10 v2 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 1/4] connector: annotate ve->cn with __rcu Vasileios Almpanis
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 2/4] connector: free the per-VE connector state after an RCU grace period Vasileios Almpanis
@ 2026-08-18 15:10 ` Vasileios Almpanis
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 4/4] proc connector: pin task VE for the exit event notification Vasileios Almpanis
3 siblings, 0 replies; 8+ messages in thread
From: Vasileios Almpanis @ 2026-08-18 15:10 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 6483e15b888e..23c66d80b6b9 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] 8+ messages in thread* [Devel] [PATCH VZ10 v2 4/4] proc connector: pin task VE for the exit event notification
2026-08-18 15:10 [Devel] [PATCH VZ10 v2 0/4] connector: do not lose exit events for in-CT listeners Vasileios Almpanis
` (2 preceding siblings ...)
2026-08-18 15:10 ` [Devel] [PATCH VZ10 v2 3/4] connector: deliver per-VE proc events under an RCU read lock Vasileios Almpanis
@ 2026-08-18 15:10 ` Vasileios Almpanis
3 siblings, 0 replies; 8+ messages in thread
From: Vasileios Almpanis @ 2026-08-18 15:10 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] 8+ messages in thread