[Devel] [PATCH VZ10] proc connector: pin task VE for the exit event notification
Konstantin Khorenko
khorenko at virtuozzo.com
Thu Aug 13 19:12:20 MSK 2026
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
>
More information about the Devel
mailing list