Skip to contentSkip to conversation

ptrace: make former thread ID available via PTRACE_GETEVENTMSG after PTRACE_EVENT_EXEC stop

PATCH Applied Denys Vlasenko 2 participants 2 messages Jun 29, 2011 – Jul 1, 2011

Subsystems (2)
VFS exec and binfmt
lore
Share & tools
lore

Denys Vlasenko

author
#0 lore raw

PATCH v4 1/1 ptrace: make former thread ID available via PTRACE_GETEVENTMSG after PTRACE_EVENT_EXEC stop

Hi Oleg,

Please take a look at version 4 of this patch.

When multithreaded program execs under ptrace, all traced threads report WIFEXITED status, except for thread group leader and the thread which execs.

Unless tracer tracks thread group relationship between tracees, which is a nontrivial task, it will not detect that execed thread no longer exists.

This patch allows tracer to figure out which thread performed this exec, by requesting PTRACE_GETEVENTMSG in PTRACE_EVENT_EXEC stop.

Another, samller problem which is solved by this patch is that tracer now can figure out which of the several concurrent execs in multithreaded program succeeded.

Signed-off-by Denys Vlasenko <[email protected]>
Signature · 1 line
vda
fs/exec.c +8 -1
index 8dca45b..55819be 100644
@@ -1357,6 +1357,7 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
 	unsigned int depth = bprm->recursion_depth;
 	int try,retval;
 	struct linux_binfmt *fmt;
+	pid_t old_pid;
 
 	retval = security_bprm_check(bprm);
 	if (retval)

@@ -1370,6 +1371,11 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
 	if (retval)
 		return retval;
 
+	/* Need to fetch pid before load_binary changes it */
+	rcu_read_lock();
+	old_pid = task_pid_nr_ns(current, task_active_pid_ns(current->parent));
+	rcu_read_unlock();
+
 	retval = -ENOENT;
 	for (try=0; try<2; try++) {
 		read_lock(&binfmt_lock);

@@ -1389,7 +1395,8 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
 			bprm->recursion_depth = depth;
 			if (retval >= 0) {
 				if (depth == 0)
-					ptrace_event(PTRACE_EVENT_EXEC, 0);
+					ptrace_event(PTRACE_EVENT_EXEC,
+							old_pid);
 				put_binfmt(fmt);
 				allow_write_access(bprm->file);
 				if (bprm->file)
Applied
#1 lore raw
Quoted 13 lines · Denys Vlasenko

When multithreaded program execs under ptrace, all traced threads report WIFEXITED status, except for thread group leader and the thread which execs.

Unless tracer tracks thread group relationship between tracees, which is a nontrivial task, it will not detect that execed thread no longer exists.

This patch allows tracer to figure out which thread performed this exec, by requesting PTRACE_GETEVENTMSG in PTRACE_EVENT_EXEC stop.

Quoted 5 lines

Another, samller problem which is solved by this patch is that tracer now can figure out which of the several concurrent execs in multithreaded program succeeded.

Signed-off-by Denys Vlasenko <[email protected]>

Thanks, applied.

Quoted 13 lines
fs/exec.c +5 -0
@@ -1370,6 +1371,11 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
 	if (retval)
 		return retval;
 
+	/* Need to fetch pid before load_binary changes it */
+	rcu_read_lock();
+	old_pid = task_pid_nr_ns(current, task_active_pid_ns(current->parent));
+	rcu_read_unlock();
+
 	retval = -ENOENT;
 	for (try=0; try<2; try++) {
 		read_lock(&binfmt_lock);

fs/exec.c int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs) · line 1399

Quoted 6 lines
fs/exec.c +2 -1
@@ -1389,7 +1395,8 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
 			bprm->recursion_depth = depth;
 			if (retval >= 0) {
 				if (depth == 0)
-					ptrace_event(PTRACE_EVENT_EXEC, 0);
+					ptrace_event(PTRACE_EVENT_EXEC,
+							old_pid);

Just for record. ->parent can be changed after we call task_pid_nr_ns(), and the new parent (tracer) can have another namespace, in this case we report the wrong pid. This is possible even now, without "PT_SEIZED implies PTRACE_EVENT_EXEC" we are going to add, although this is very unlikely and in this case PTRACE_EVENT_EXEC is spurious anyway. But when we change the behaviour of PT_SEIZED, this race becomes not that exotic, although very unlikely anyway.

I do not think we should try to fix this, it is not trivial and doesn't worth the trouble.

Oleg.