[Devel] [PATCH VZ10] proc connector: pin task VE for the exit event notification

[Devel] [PATCH VZ10] proc connector: pin task VE for the exit event notification

[Vasileios Almpanis]

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     |  7 +++++--
 kernel/exit.c               |  5 ++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index d41c8aca1866..cd75f1820914 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -362,9 +362,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..0e545774aa81 100644
--- a/include/linux/cn_proc.h
+++ b/include/linux/cn_proc.h
@@ -17,6 +17,7 @@
 #ifndef CN_PROC_H
 #define CN_PROC_H
 
+#include <linux/ve.h>
 #include <uapi/linux/cn_proc.h>
 
 /*
@@ -36,7 +37,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 +63,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);

---
base-commit: 209f11f2c454a88761fb8f0a820fa22b6feae720
change-id: 20260813-connectors-3571be4ae57b

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

Re: [Devel] [PATCH VZ10] proc connector: pin task VE for the exit event notification

[Konstantin Khorenko]

On 8/13/26 13:13, Vasileios Almpanis wrote:
...
> diff --git a/include/linux/cn_proc.h b/include/linux/cn_proc.h
> index 9701c13d82df..0e545774aa81 100644
> --- a/include/linux/cn_proc.h
> +++ b/include/linux/cn_proc.h
> @@ -17,6 +17,7 @@
>  #ifndef CN_PROC_H
>  #define CN_PROC_H
>  
> +#include <linux/ve.h>

May be we can put just a ve_struct forward declaration here, not putting a heavy ve.h here?

>  #include <uapi/linux/cn_proc.h>
>  
>  /*
> @@ -36,7 +37,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 +63,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);

In brief:
=========
Can we dereference a NULL ve->ve_nsproxy, or touch a freed ve->cn after that?

Ok, we have grabbed the VE, the css reference taken by get_task_ve() keeps ve_struct itself alive, but ve->cn and ve->ve_nsproxy are torn down by ve_exit_ns() independently of that reference.

drivers/connector/cn_proc.c:fill_exit_event() uses ve->ve_nsproxy later which seems not guarded by anything.
And drivers/connector/cn_proc.c:send_msg_ve() uses ve->cn.

Detailed:
=========
kernel/ve/ve.c:ve_exit_ns() {
        ...
        cgroup_unmark_ve_roots(ve);
        ve_hook_iterate_fini(VE_SS_CHAIN, ve);  /* -> cn_fini_ve() */
        ve_list_del(ve);
        ve_drop_context(ve);                    /* ve_nsproxy = NULL */
        ve_set_state(ve, VE_STATE_STOPPED);
        ...
}

drivers/connector/connector.c:cn_fini_ve() {
        ...
        cn_proc_fini_ve(ve);            /* free_percpu(ve->cn->local_event) */
        ...
        netlink_kernel_release(dev->nls);
        kfree(ve->cn);
        ve->cn = NULL;
}

The only guard on the send path is the ve->cn test:

drivers/connector/cn_proc.c:proc_event_num_listeners() {
        if (ve->cn)
                return atomic_read(&ve->cn->proc_event_num_listeners);
        return 0;
}

Note that this guard does not filter the container-stop case out: the
counter is only decremented on an explicit PROC_CN_MCAST_IGNORE message in
cn_proc_mcast_ctl().  When a listener dies without sending it (e.g. it is
killed by zap_pid_ns_processes() during container stop), cn_release()
frees sk_user_data but leaves the counter alone:

drivers/connector/connector.c:cn_release() {
        if (groups && test_bit(CN_IDX_PROC - 1, groups)) {
                kfree(sk->sk_user_data);
                sk->sk_user_data = NULL;
        }
}

so any container that ever had an in-container listener keeps the counter
at >= 1 up to and including the teardown window.

After the guard passes, both ve->ve_nsproxy and ve->cn are used without
holding anything:

drivers/connector/cn_proc.c:fill_exit_event() {
        struct pid_namespace *pid_ns = ve->ve_nsproxy->pid_ns_for_children;
        ...
}

drivers/connector/cn_proc.c:send_msg_ve() {
        ...
        local_lock(&ve->cn->local_event->lock);
        ...
        cn_netlink_send_mult_ve(ve, msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
                                cn_filter, (void *)filter_data);
        ...
}

Since the ve is now captured before exit_notify(), the reaped task keeps
using its container ve instead of falling back to ve0, and the reap is
exactly what unblocks the teardown.  release_task() wakes the container
init from inside the exiting task:

kernel/pid.c:free_pid() {
        ...
                switch (--ns->pid_allocated) {
                case 2:
                case 1:
                        /* When all that is left in the pid namespace
                         * is the reaper wake up the reaper.  The reaper
                         * may be sleeping in zap_pid_ns_processes().
                         */
                        wake_up_process(ns->child_reaper);
        ...
}

so the sequence looks reachable:

cpu0, last container task
  do_exit()
    ve = get_task_ve(tsk);              /* container ve, css ref */
    exit_notify()
      release_task(tsk)
        __exit_signal() -> free_pid()   /* wakes container init */
        proc_flush_pid()                /* can sleep here */
        exit_ve_namespace(tsk)          /* tsk->task_ve = ve0 */
    proc_exit_connector(tsk, &pids, ve)
      proc_event_num_listeners(ve)      /* ve->cn still set, listeners > 0 */
      cn_msg_fill()
        fill_exit_event()
          ve->ve_nsproxy->pid_ns_for_children

cpu1, container init woken by free_pid()
  zap_pid_ns_processes()
    ve_exit_ns()
      cn_fini_ve()                      /* kfree(ve->cn), ve->cn = NULL */
      ve_drop_context()                 /* ve->ve_nsproxy = NULL */

The two reads of ve->cn and ve->ve_nsproxy are not covered by a common
lock or rcu section, so nothing appears to stop the teardown from
completing in between them.

Before this change the container ve could only be observed while the task
was still hashed, which kept zap_pid_ns_processes() waiting on
pid_allocated and therefore kept ve_exit_ns() from starting, so the
already-reaped state did not reach this code.  The pre VE-namespace
ve_exit() had an explicit guard for the same concern:

        if (!ve->ve_nsproxy ||
            !task_pid_nr_ns(task, ve->ve_nsproxy->pid_ns_for_children))
                rcu_assign_pointer(task->task_ve, &ve0);

Would fill_exit_event() need to read ve_nsproxy the way task_pid_ve_nr()
does, under rcu_read_lock() with rcu_dereference() and a NULL test, and
bail out of the container send when it is gone?

>  	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);
> 
> ---
> base-commit: 209f11f2c454a88761fb8f0a820fa22b6feae720
> change-id: 20260813-connectors-3571be4ae57b
> 

Re: [Devel] [PATCH VZ10] proc connector: pin task VE for the exit event notification

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

On 8/13/26 13:13, Vasileios Almpanis wrote:
> 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     |  7 +++++--
>  kernel/exit.c               |  5 ++++-
>  3 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
> index d41c8aca1866..cd75f1820914 100644
> --- a/drivers/connector/cn_proc.c
> +++ b/drivers/connector/cn_proc.c
> @@ -362,9 +362,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..0e545774aa81 100644
> --- a/include/linux/cn_proc.h
> +++ b/include/linux/cn_proc.h
> @@ -17,6 +17,7 @@
>  #ifndef CN_PROC_H
>  #define CN_PROC_H
>  
> +#include <linux/ve.h>
>  #include <uapi/linux/cn_proc.h>
>  
>  /*
> @@ -36,7 +37,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 +63,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);
> 
> ---
> base-commit: 209f11f2c454a88761fb8f0a820fa22b6feae720
> change-id: 20260813-connectors-3571be4ae57b
> 

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.