(for both patches)
Reviewed-by: Pavel Tikhomirov <ptikhomi...@virtuozzo.com>
On 7/21/25 19:29, Konstantin Khorenko wrote:
After do_exit() -> exit_notify() process pid and tgid may be detached
(free_pid()) before proc_exit_connector() call happens, thus
proc_exit_connector() fail to report proper pid/tgid via netlink.
Ordinary "good" scenario:
parent 37311 [000] 22305.315986: probe:alloc_pid_L85: child_process_pid=38210
ffffffffa4f32888 alloc_pid+0x138
ffffffffa4f07850 copy_process+0xc20
ffffffffa4f084cb kernel_clone+0x9b
ffffffffa4f08980 __do_sys_clone+0x60
ffffffffa5a9e45c do_syscall_64+0x5c
ffffffffa5c0012f entry_SYSCALL_64+0xaf
1189d7 _Fork+0x27 (/usr/lib64/libc.so.6)
// proc_event_connector_ve() is executed BEFORE parent gets SIGCHLD
child 38210 [000] 22305.316156: probe:fill_exit_event_L7:
ffffffffa5616568 fill_exit_event+0x38
(/usr/lib/debug/lib/modules/5.14.0-427.55.1.vz9.82.9/vmlinux)
ffffffffa5616a51 proc_event_connector_ve+0xa1
(/usr/lib/debug/lib/modules/5.14.0-427.55.1.vz9.82.9/vmlinux)
ffffffffa4f0fe7f do_exit+0x2cf
(/usr/lib/debug/lib/modules/5.14.0-427.55.1.vz9.82.9/vmlinux)
ffffffffa4f1018d do_group_exit+0x2d
(/usr/lib/debug/lib/modules/5.14.0-427.55.1.vz9.82.9/vmlinux)
// parents gets SIGCHLD AFTER data for netlink listeners is filled, child
pid/tgid is detached
parent 37311 [001] 22305.316162: probe:free_pid_L9: child_process_pid=38210
ffffffffa4f32637 free_pid+0x57
ffffffffa4f0db11 __exit_signal+0x2d1
ffffffffa4f0df25 release_task+0xc5
ffffffffa4f0e73a wait_task_zombie+0x14a
ffffffffa4f0f00a do_wait+0x16a
ffffffffa4f10336 kernel_wait4+0xa6
Possible "bad" scenario:
parent 37311 [001] 22305.316371: probe:alloc_pid_L85:
child_process_pid=38211
ffffffffa4f32888 alloc_pid+0x138
ffffffffa4f07850 copy_process+0xc20
ffffffffa4f084cb kernel_clone+0x9b
ffffffffa4f08980 __do_sys_clone+0x60
ffffffffa5a9e45c do_syscall_64+0x5c
ffffffffa5c0012f entry_SYSCALL_64+0xaf
1189d7 _Fork+0x27 (/usr/lib64/libc.so.6)
// parents gets SIGCHLD BEFORE data for netlink listeners is filled, child
pid/tgid is detached
parent 37311 [000] 22305.316552: probe:free_pid_L9:
child_process_pid=38211
ffffffffa4f32637 free_pid+0x57
ffffffffa4f0db11 __exit_signal+0x2d1
ffffffffa4f0df25 release_task+0xc5
ffffffffa4f0e73a wait_task_zombie+0x14a
ffffffffa4f0f00a do_wait+0x16a
ffffffffa4f10336 kernel_wait4+0xa6
// proc_event_connector_ve() is executed AFTER parent gets SIGCHLD,
// cannot find pid/tgid in idr anymore
:-1 -1 [001] 22305.316560: probe:fill_exit_event_L7: (ffffffffa5616567)
pid_ns=0xffffffffa6a5efa0 process_pid=0
ffffffffa5616568 fill_exit_event+0x38
ffffffffa5616a51 proc_event_connector_ve+0xa1
ffffffffa4f0fe7f do_exit+0x2cf
ffffffffa4f1018d do_group_exit+0x2d
The issue is caught by LTP suite_connectors.exec.cn_pec_sh test, so the
reproducer is:
// install/build LTP
cd /opt/ltp/testcases/bin/
export PATH=$PATH:/opt/ltp/testcases/bin
for i in $(seq 1 1024); do echo -n "$i: "; TST_TIMEOUT=100 cn_pec.sh 2>&1 | \
tee run.log | egrep "failed.*0" || break; done; cat run.log
Solve this by saving process pid/tgid before exit_notify() is called and
use them in proc_exit_connector().
https://virtuozzo.atlassian.net/browse/PSBM-161075
Fixes: 858c48a22fb2 ("proc connector: add pid namespace awareness")
Signed-off-by: Konstantin Khorenko <khore...@virtuozzo.com>
Feature: cgroup: control group rules daemon (cgrulesengd) virtualization
---
drivers/connector/cn_proc.c | 11 ++++++-----
include/linux/cn_proc.h | 13 +++++++++++--
kernel/exit.c | 7 ++++++-
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 3349b6336e9f..2d1bad855357 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -292,13 +292,14 @@ 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 unused)
+ struct task_struct *task, long cookie_pids)
{
struct pid_namespace *pid_ns = ve->ve_ns->pid_ns_for_children;
struct task_struct *parent;
+ struct pids *pids = (struct pids *)cookie_pids;
- ev->event_data.exit.process_pid = task_pid_nr_ns(task, pid_ns);
- ev->event_data.exit.process_tgid = task_tgid_nr_ns(task, pid_ns);
+ 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);
ev->event_data.exit.exit_code = task->exit_code;
ev->event_data.exit.exit_signal = task->exit_signal;
@@ -314,9 +315,9 @@ static bool fill_exit_event(struct proc_event *ev, struct ve_struct *ve,
return true;
}
-void proc_exit_connector(struct task_struct *task)
+void proc_exit_connector(struct task_struct *task, struct pids *pids)
{
- proc_event_connector(task, PROC_EVENT_EXIT, 0, fill_exit_event);
+ proc_event_connector(task, PROC_EVENT_EXIT, (long)pids,
fill_exit_event);
}
/*
diff --git a/include/linux/cn_proc.h b/include/linux/cn_proc.h
index 1d5b02a96c46..9701c13d82df 100644
--- a/include/linux/cn_proc.h
+++ b/include/linux/cn_proc.h
@@ -19,6 +19,15 @@
#include <uapi/linux/cn_proc.h>
+/*
+ * The struct is used solely for pinning task pids for proc connector
+ * notification on process exit.
+ */
+struct pids {
+ struct pid *pid;
+ struct pid *tgid;
+};
+
#ifdef CONFIG_PROC_EVENTS
void proc_fork_connector(struct task_struct *task);
void proc_exec_connector(struct task_struct *task);
@@ -27,7 +36,7 @@ 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);
+void proc_exit_connector(struct task_struct *task, struct pids *pids);
#else
static inline void proc_fork_connector(struct task_struct *task)
{}
@@ -52,7 +61,7 @@ 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)
+static inline void proc_exit_connector(struct task_struct *task, struct pids
*pids)
{}
#endif /* CONFIG_PROC_EVENTS */
#endif /* CN_PROC_H */
diff --git a/kernel/exit.c b/kernel/exit.c
index 7273cecde8ce..717013bd378e 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -788,6 +788,7 @@ void __noreturn do_exit(long code)
{
struct task_struct *tsk = current;
int group_dead;
+ struct pids pids;
WARN_ON(tsk->plug);
@@ -868,8 +869,12 @@ void __noreturn do_exit(long code)
flush_ptrace_hw_breakpoint(tsk);
exit_tasks_rcu_start();
+ pids.pid = get_pid(task_pid(tsk));
+ pids.tgid = get_pid(task_tgid(tsk));
exit_notify(tsk, group_dead);
- proc_exit_connector(tsk);
+ proc_exit_connector(tsk, &pids);
+ put_pid(pids.tgid);
+ put_pid(pids.pid);
mpol_put_task_policy(tsk);
#ifdef CONFIG_FUTEX
if (unlikely(current->pi_state_cache))
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
