ptrace,signal: Fix notifications to the real parent while ptraced

Hello,

This patchset implements "P2. Fix notifications to the real parent" of the ptrace job control improvements proposal[1].

As the whole job control / ptrace logic is quite delicate, I tried to be very granual with changes and add plenty of explanations for subtleties.

This patchset contains the following eight patches.

0001-job-control-Don-t-set-group_stop-exit_code-if-re-ent.patch 0002-job-control-Small-reorganization-of-wait_consider_ta.patch 0003-job-control-Fix-ptracer-wait-2-hang-and-explain-nota.patch 0004-job-control-Allow-access-to-job-control-events-throu.patch 0005-job-control-Add-for_ptrace-to-do_notify_parent_cldst.patch 0006-job-control-Job-control-stop-notifications-should-al.patch 0007-job-control-Notify-the-real-parent-of-job-control-ev.patch 0008-job-control-Don-t-send-duplicate-job-control-stop-no.patch

0001-0003 fix subtle issues and prepare for wait(2) related changes. I don't think any of this needs to be backported. 0004 make the job control stopped/continued states visible to the real parent regardless of ptrace.

0005-0006 prepare for notification related changes. 0007-0008 enable proper job control notifications to the real parent regardless of ptrace.

This patchset introduces behavior changes; however, all the changes are visible only to the real parent. As basically everything job control related was broken while ptraced, I don't think these changes need to be conditionalized (ie. enabled only when explicitly told so). Doing so is likely to only increase confusion with few benefits.

After this patchset, I can ^Z a strace(1) attached command and get a shell prompt. The interaction of course is not perfect yet because strace(1) currently unconditionally overrides job control stop, but it's a progress. For strace(1) itself, nothing really changes.

This patchset is based on 2.6.38-rc7 (212e3499b2) + "group stop / ptrace updates, take#2" patchset[2] which implements P1. The git branch for the P1 patchset has been regenerated with Oleg's Acked-by's added and available in ptrace-review-P1 branch.

This patchset is available in the following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git ptrace-review-P2

HEAD is fbbe61c532ba23d2d00bff7c2b354e0522ae4afc. git.korg takes some time to sync so if it shows older commit, please try again after a while. diffstat follows.

 kernel/exit.c   |   83 +++++++++++++++++++++++++++++++---------
 kernel/signal.c |  116 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 170 insertions(+), 29 deletions(-)

Thanks.

--
tejun

[1] http://thread.gmane.org/gmane.linux.kernel/1107045
[2] http://thread.gmane.org/gmane.linux.kernel/1109224

While ptraced, a task may be resumed while the containing process is still job control stopped. If the task receives another stop signal in this state, it will still initiate group stop, which generates group_exit_code, which the real parent would be able to see once the ptracer detaches.

In this scenario, the real parent may see two consecutive CLD_STOPPED events from two stop signals without intervening SIGCONT, which normally is impossible.

Test case follows.

#include <stdio.h>
#include <unistd.h>
#include <sys/ptrace.h>
#include <sys/wait.h>

int main(void)

{
	pid_t tracee;
	siginfo_t si;

	tracee = fork();
	if (!tracee)
		while (1)
			pause();

	kill(tracee, SIGSTOP);
	waitid(P_PID, tracee, &si, WSTOPPED);

	if (!fork()) {
		ptrace(PTRACE_ATTACH, tracee, NULL, NULL);
		waitid(P_PID, tracee, &si, WSTOPPED);
		ptrace(PTRACE_CONT, tracee, NULL, (void *)(long)si.si_status);
		waitid(P_PID, tracee, &si, WSTOPPED);
		ptrace(PTRACE_CONT, tracee, NULL, (void *)(long)si.si_status);
		waitid(P_PID, tracee, &si, WSTOPPED);
		ptrace(PTRACE_DETACH, tracee, NULL, NULL);
		return 0;
	}

	while (1) {
		si.si_pid = 0;
		waitid(P_PID, tracee, &si, WSTOPPED | WNOHANG);
		if (si.si_pid)
			printf("st=%02d c=%02d\n", si.si_status, si.si_code);
	}
	return 0;
}

Before the patch, the latter waitid() in polling mode reports the second stopped event generated by the implied SIGSTOP of PTRACE_ATTACH.

st=19 c=05
^C

After the patch, the second event is not reported.

---
 kernel/signal.c |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

index c146150..6130cb8 100644
@@ -1827,10 +1827,27 @@ static int do_signal_stop(int signr)
 		    unlikely(signal_group_exit(sig)))
 			return 0;
 		/*
-		 * There is no group stop already in progress.
-		 * We must initiate one now.
+		 * There is no group stop already in progress.  We must
+		 * initiate one now.
+		 *
+		 * While ptraced, a task may be resumed while group stop is
+		 * still in effect and then receive a stop signal and
+		 * initiate another group stop.  This deviates from the
+		 * usual behavior as two consecutive stop signals can't
+		 * cause two group stops when !ptraced.
+		 *
+		 * The condition can be distinguished by testing whether
+		 * SIGNAL_STOP_STOPPED is already set.  Don't generate
+		 * group_exit_code in such case.
+		 *
+		 * This is not necessary for SIGNAL_STOP_CONTINUED because
+		 * an intervening stop signal is required to cause two
+		 * continued events regardless of ptrace.
 		 */
-		sig->group_exit_code = signr;
+		if (!(sig->flags & SIGNAL_STOP_STOPPED))
+			sig->group_exit_code = signr;
+		else
+			WARN_ON_ONCE(!task_ptrace(current));
 
 		current->group_stop &= ~GROUP_STOP_SIGMASK;
 		current->group_stop |= signr | gstop;

1.7.1

Hi Tejun,

I hope you still remember you sent these patches, perhaps you can even recall what they should do ;)

		for (t = next_thread(current); t != current;
		     t = next_thread(t)) {
			t->group_stop &= ~GROUP_STOP_SIGMASK;
			/*
			 * Setting state to TASK_STOPPED for a group
			 * stop is always done with the siglock held,
			 * so this check has no races.
			 */
			if (!(t->flags & PF_EXITING) && !task_is_stopped(t)) {
				t->group_stop |= signr | gstop;
				sig->group_stop_count++;
				signal_wake_up(t, 0);
			} else {
				task_clear_group_stop_pending(t);
			}
		}

Hey, Oleg.

Yeah, was about to ping you actually. BTW, I biked for a couple of hours and had a beer and am feeling pretty stupid at the moment, so my chance of writing something stupid is higher than usual.

I see. It can't happen for PTRACE_DETACH due to ptrace_check_attach() but can if the debugger is exiting.

Right, the task_is_traced() tests. I agree that the culprit is __ptrace_unlink(). It should always put the tracee in the appropriate state. I'll prep a patch for this.

Hmmm... I'm not sure whether this matters. If the group stop hasn't completed yet, I don't think we need to guarantee which one is reported. With single thread, the condition can't happen and, with multithread, the userland has no way to tell which one of the two signals actually started group stop so we can report any. The ordering is undefined.

It might be cleaner that way but I think the current code could be easier to explain. Mights and coulds.

Hmm... I think this is from the older implementation where task_clear_group_stop_pending() did more than just clearing the two flags. We can remove it.

GROUP_STOP_SIGMASK only affects ptracers. The extra stop signal is visible to one ptracer and possibly other ptracers too, so I think it's more appropriate to report the newest signal. Please consider the following scenario.

* Two threads in a process. SIGNAL_STOP_STOPPED.

* PTRACE_ATTACH on both threads and both are PTRACE_CONT'd.

* A stopsig is delivered to one of them which initiates group stop.

* Both ptracers notice the tracees participating in the group stop.

I think there was a reason why it couldn't be a local variable. Let's see if I can remember it. Ah, okay, please consider the following scenario. This one needs to be documented.

* A thread does group stop and the parent consumed exit code.

* ptracer attaches and sees the group stop signal.

* PTRACE_CONT and the thread leaves do_signal_stop().

* PTRACE_DETACH. The thread returns to do_signal_stop() and re-enters TASK_STOPPED.

* Another ptracer does PTRACE_ATTACH.

The second ptracer wants to know the signo too but if it were stored in a local variable, it wouldn't be available anywhere.

Isn't this addressed by the last patch in this thread?

This was changed by "signal: Use GROUP_STOP_PENDING to stop once for a single group stop". The previous behavior was inconsistent. If the tracee was running it would get notified; otherwise, not. Also, the tracer could be notified multiple times for the same group stop. I think the new behavior is sane and falls within the boundaries set by the inconsistencies of the previous behavior.

Thanks.

tejun

I agree, this is minor, but

Yes, if two threads dequeue the sig_kernel_stop() signals, the ordering is undefined. But ptrace should not change sig->group_exit_code once it was already set by the thread which starts the group stop.

Suppose that debugger PTRACE_CONTs the stopped thread and then it gets SIGSTOP and calls do_signal_stop() again. In theory this all is possible before SIGNAL_STOP_STOPPED. This can confuse real_parent. Say, real_parent itself sends SIGTTIN to the child and naturally expects WIFSTOPPED() == SIGTTIN.

And btw, I think we should not change/set ->group_stop_count if

But again, even if I am right this is minor and we can change this later. And btw, I think we should not change/set ->group_stop_count if SIGNAL_STOP_STOPPED || group_stop_count. We will see.

IOW. In short - you can skip the whole email. I do not see any real problems which could delay this patch.

OK, good.

Sure, this is correct. But I meant another case. In the scenario above, suppose that only one thread was PTRACE_CONT'ed.

Again, again, again. This is very minor. I do not mean it makes sense to change this, at least right now. Just I have a vague feeling we can cleanup this a little bit later.

For example. Ignoring task_participate_group_stop(), why task_clear_group_stop_pending() preserves the GROUP_STOP_SIGMASK ? This doesn't hurt, sure. But looks a bit inconsistent.

Anyway, please forget. Right now this is off-topic.

Yes, sure. But this is basically the same case. My point was, this signr must be correct inside the retry loop, otherwise we could just report SIGSTOP because we can't guarantee the "correct" signr anyway.

Let's look at your example again. Suppose that the process was stopped by SIGSTOP.

When the first ptracer attaches, each thread correctly reports SIGSTOP. But if it plays with PTRACE_CONT and then detaches, the next ptracer can see, say, SIGTTIN. And different threads can report different signals.

Again, again, again. I do not want to delay this changes. But _perhaps_ we can cleanup this a bit later.

OK, lets keep GROUP_STOP_SIGMASK. Then, perhaps, we should never change "group_stop & GROUP_STOP_SIGMASK" if it is non-zero. PTRACE_CONT should clear it.

The only special case is stop->PTRACE_CONT->do_signal_stop(). In this case the tracee should keep its "group_stop & GROUP_STOP_SIGMASK" but use "int signr" passed to do_signal_stop() as an argument for ptrace_stop().

Not that I seriously think this needs the attention. In any case, not now.

Yes, it was.

Not sure I understand... We are setting GROUP_STOP_PENDING | CONSUME again. T2 has already reported ptrace_stop(CLD_STOPPED) to the tracer. It is stopped. Now it will report another CLD_STOPPED after PTRACE_CONT.

Oleg.

Hello,

Hmmm... There are two competing signals in that case - SIGTTIN sent by the parent and SIGSTOP sent by someone else. The parent doesn't have any control over which signal wins the race nor is it visible in any way before the group stop is complete. The two signals are equivalent.

I can't think of a scenario where the parent would be able to differentiate the two signals (in the sense that it can say the latter is the wrong signal). If you can, please share.

Yeah, maybe. We would be suppressing group stop completion anyway so the count doesn't make whole lot of difference. The only difference would be that group stop completion would be delayed if stop signals are continuously delievered by ptracee's in the thread group. I don't think it matters one way or the other.

Yeah, it probably is better to clear the signo too.

I think it depends on how you look at it. Group stop behavior is different under ptrace anyway. We allow PTRACE_CONT'd tracees to re-initiate and re-enter the existing group stop. Which signo is the right one to report is debatable, but,

* For the tracee which delivers the signal which re-initiates the existing group stop, it would be just weird to report different signo from the one delivered.

* It would also be weird to report different signo for other tracees in the group depending on whether they were running at the time of re-initiation or not.

So, I think just defining the stop signo to be reported while ptraced as "the latest group stop signal delivered" isn't too bad. It's simple and more consistent with the existing behavior.

Okay, I see. Maybe we should discern between traced for group stop from other traps but then again given the group stop re-entering while ptraced it can be considered a relatively consistent behavior. Yeah, but probably better to remove the double reporting.

Thanks.

tejun

"someone else" can be PTRACE_CONT(SIGSTOP) from the debugger.

I didn't mean this is really wrong or can lead to some problems. Just this looks inconsistent a bit.

Yes. I have a vague feeling a new GROUP_STOP_YES_I_AM_STOPPED can be useful anyway. It should be set by task_participate_group_stop() if the task participates. We will see.

Oleg.

Hey,

The debugger should be doing PTRACE_CONT(notified_signo). Am I missing something? Anyways, this doesn't really matter. I kinda like just testing STOP_STOPPED both in stop initiation and notification paths but well it's no biggie either way.

Yeah, agreed. Let's mark it and ensure that a task doesn't stop for group stop back-to-back.

Thanks.

tejun

Why? it can change the signal.

OK, agreed, please forget.

Oleg.

Hello,

Yeah, sure, it can but then it gets to live with the consequences, which is the changed signal being reported. In fact, in such case, I think reporting the changed signal is the right thing to do(tm).

Thanks.

tejun

Move EXIT_DEAD test in wait_consider_task() above ptrace check. As ptraced tasks can't be EXIT_DEAD, this change doesn't cause any behavior change. This is to prepare for further changes.

---
 kernel/exit.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

index f9a45eb..b4a935c 100644
@@ -1537,6 +1537,10 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 		return 0;
 	}
 
+	/* dead body doesn't have much to contribute */
+	if (p->exit_state == EXIT_DEAD)
+		return 0;
+
 	if (likely(!ptrace) && unlikely(task_ptrace(p))) {
 		/*
 		 * This child is hidden by ptrace.

@@ -1546,9 +1550,6 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 		return 0;
 	}
 
-	if (p->exit_state == EXIT_DEAD)
-		return 0;
-
 	/*
 	 * We don't reap group leaders with subthreads.
 	 */

1.7.1

wait(2) and friends allow access to stopped/continued states through zombies, which is required as the states are process-wide and should be accessible whether the leader task is alive or undead. wait_consider_task() implements this by always clearing notask_error and going through wait_task_stopped/continued() for unreaped zombies.

However, while ptraced, the stopped state is per-task and as such if the ptracee became a zombie, there's no further stopped event to listen to and wait(2) and friends should return -ECHILD on the tracee.

Fix it by clearing notask_error only if WCONTINUED is set for ptraced zombies. While at it, document why clearing notask_error is safe for each case.

Test case follows.

  #include <stdio.h>
  #include <unistd.h>
  #include <pthread.h>
  #include <time.h>
  #include <sys/types.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

static void *nooper(void *arg)

  {
	  pause();
	  return NULL;

}

int main(void)

  {
	  const struct timespec ts1s = { .tv_sec = 1 };
	  pid_t tracee, tracer;
	  siginfo_t si;

	  tracee = fork();
	  if (tracee == 0) {
		  pthread_t thr;

		  pthread_create(&thr, NULL, nooper, NULL);
		  nanosleep(&ts1s, NULL);
		  printf("tracee exiting\n");
		  pthread_exit(NULL);	/* let subthread run */
	  }

	  tracer = fork();
	  if (tracer == 0) {
		  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);
		  while (1) {
			  if (waitid(P_PID, tracee, &si, WSTOPPED) < 0) {
				  perror("waitid");
				  break;
			  }
			  ptrace(PTRACE_CONT, tracee, NULL,
				 (void *)(long)si.si_status);
		  }
		  return 0;
	  }

	  waitid(P_PID, tracer, &si, WEXITED);
	  kill(tracee, SIGKILL);
	  return 0;

}

Before the patch, after the tracee becomes a zombie, the tracer's waitid(WSTOPPED) never returns and the program doesn't terminate.

tracee exiting
^C

After the patch, tracee exiting triggers waitid() to fail.

tracee exiting
waitid: No child processes

---
 kernel/exit.c |   44 ++++++++++++++++++++++++++++++++++----------
 1 files changed, 34 insertions(+), 10 deletions(-)

index b4a935c..7171821 100644
@@ -1550,17 +1550,41 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 		return 0;
 	}
 
-	/*
-	 * We don't reap group leaders with subthreads.
-	 */
-	if (p->exit_state == EXIT_ZOMBIE && !delay_group_leader(p))
-		return wait_task_zombie(wo, p);
+	/* slay zombie? */
+	if (p->exit_state == EXIT_ZOMBIE) {
+		/* we don't reap group leaders with subthreads */
+		if (!delay_group_leader(p))
+			return wait_task_zombie(wo, p);
 
-	/*
-	 * It's stopped or running now, so it might
-	 * later continue, exit, or stop again.
-	 */
-	wo->notask_error = 0;
+		/*
+		 * Allow access to stopped/continued state via zombie by
+		 * falling through.  Clearing of notask_error is complex.
+		 *
+		 * When !@ptrace:
+		 *
+		 * If WEXITED is set, notask_error should naturally be
+		 * cleared.  If not, subset of WSTOPPED|WCONTINUED is set,
+		 * so, if there are live subthreads, there are events to
+		 * wait for.  If all subthreads are dead, it's still safe
+		 * to clear - this function will be called again in finite
+		 * amount time once all the subthreads are released and
+		 * will then return without clearing.
+		 *
+		 * When @ptrace:
+		 *
+		 * Stopped state is per-task and thus can't change once the
+		 * target task dies, so notask_error should be cleared only
+		 * if WCONTINUED is set.
+		 */
+		if (likely(!ptrace) || (wo->wo_flags & WCONTINUED))
+			wo->notask_error = 0;
+	} else {
+		/*
+		 * @p is alive and it's gonna stop, continue or exit, so
+		 * there always is something to wait for.
+		 */
+		wo->notask_error = 0;
+	}
 
 	if (task_stopped_code(p, ptrace))
 		return wait_task_stopped(wo, ptrace, p);

1.7.1

Yes. The program should terminate after nooper() exits. Agreed, this looks strange.

OK, but

	if (p->exit_state == EXIT_ZOMBIE) {
		if (!delay_group_leader(p))
			return wait_task_zombie(wo, p);

		ptrace_unlink();
		wait_task_zombie(WNOWAIT);
	}

That comment is for !@ptrace, so the code and comment are in sync, but yes, if WEXITED is set, notask_error better be cleared regardless of @ptrace.

Interesting point. Yeah, I agree. wait(WEXITED) from the ptracer should only wait for the tracee itself, not the group. When they are one and the same, I don't think we need to do anything differently from now.

If we change the behavior that way, it would also fit better with the rest of the new behavior where the real parent and ptracer have separate roles when wait(2)ing for stopped states.

The question is how the change would affect the existing users. When the debugee is a direct child, nothing will change. When attaching to a separate group, I don't think it even matters. Does gdb handle group leader any differently from the rest when attached to an unrelated group?

Thanks.

tejun

Just in case... But of course I didn't mean this patch should be updated to handle the EXIT_ZOMBIE case.

Yes, of course. Perhaps we can never do this.

Actually, I think this is the most problematic case... Perhaps it would be safer to add WEXITED_THREAD for ptrace. I dunno.

gdb certainly has some problems with the dead leaders. But I can't recall what exactly. Will try to check later...

In any case, I only tried to discuss what else we can do with the current strange semantics. When it comes to ptrace, group_leader should not represent the whole process.

Oleg.

Argh. please ingnore this part. I misread the code above.

Still true, or I missed something again...

Oleg.

wait(2) and friends allow access to stopped/continued states through zombies, which is required as the states are process-wide and should be accessible whether the leader task is alive or undead. wait_consider_task() implements this by always clearing notask_error and going through wait_task_stopped/continued() for unreaped zombies.

However, while ptraced, the stopped state is per-task and as such if the ptracee became a zombie, there's no further stopped event to listen to and wait(2) and friends should return -ECHILD on the tracee.

Fix it by clearing notask_error only if WCONTINUED | WEXITED is set for ptraced zombies. While at it, document why clearing notask_error is safe for each case.

Test case follows.

  #include <stdio.h>
  #include <unistd.h>
  #include <pthread.h>
  #include <time.h>
  #include <sys/types.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

static void *nooper(void *arg)

  {
	  pause();
	  return NULL;

}

int main(void)

  {
	  const struct timespec ts1s = { .tv_sec = 1 };
	  pid_t tracee, tracer;
	  siginfo_t si;

	  tracee = fork();
	  if (tracee == 0) {
		  pthread_t thr;

		  pthread_create(&thr, NULL, nooper, NULL);
		  nanosleep(&ts1s, NULL);
		  printf("tracee exiting\n");
		  pthread_exit(NULL);	/* let subthread run */
	  }

	  tracer = fork();
	  if (tracer == 0) {
		  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);
		  while (1) {
			  if (waitid(P_PID, tracee, &si, WSTOPPED) < 0) {
				  perror("waitid");
				  break;
			  }
			  ptrace(PTRACE_CONT, tracee, NULL,
				 (void *)(long)si.si_status);
		  }
		  return 0;
	  }

	  waitid(P_PID, tracer, &si, WEXITED);
	  kill(tracee, SIGKILL);
	  return 0;

}

Before the patch, after the tracee becomes a zombie, the tracer's waitid(WSTOPPED) never returns and the program doesn't terminate.

tracee exiting
^C

After the patch, tracee exiting triggers waitid() to fail.

tracee exiting
waitid: No child processes

-v2: Oleg pointed out that exited in addition to continued can happen
for ptraced dead group leader. Clear notask_error for ptraced child on WEXITED too.

---
WEXITED bug fixed. Let's tackle the ptraced per-task wait(2) thing later. Thanks.

 kernel/exit.c |   44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

Index: work/kernel/exit.c
===================================================================
@@ -1550,17 +1550,41 @@ static int wait_consider_task(struct wai
 		return 0;
 	}
 
-	/*
-	 * We don't reap group leaders with subthreads.
-	 */
-	if (p->exit_state == EXIT_ZOMBIE && !delay_group_leader(p))
-		return wait_task_zombie(wo, p);
+	/* slay zombie? */
+	if (p->exit_state == EXIT_ZOMBIE) {
+		/* we don't reap group leaders with subthreads */
+		if (!delay_group_leader(p))
+			return wait_task_zombie(wo, p);
 
-	/*
-	 * It's stopped or running now, so it might
-	 * later continue, exit, or stop again.
-	 */
-	wo->notask_error = 0;
+		/*
+		 * Allow access to stopped/continued state via zombie by
+		 * falling through.  Clearing of notask_error is complex.
+		 *
+		 * When !@ptrace:
+		 *
+		 * If WEXITED is set, notask_error should naturally be
+		 * cleared.  If not, subset of WSTOPPED|WCONTINUED is set,
+		 * so, if there are live subthreads, there are events to
+		 * wait for.  If all subthreads are dead, it's still safe
+		 * to clear - this function will be called again in finite
+		 * amount time once all the subthreads are released and
+		 * will then return without clearing.
+		 *
+		 * When @ptrace:
+		 *
+		 * Stopped state is per-task and thus can't change once the
+		 * target task dies.  Only continued and exited can happen.
+		 * Clear notask_error if WCONTINUED | WEXITED.
+		 */
+		if (likely(!ptrace) || (wo->wo_flags & (WCONTINUED | WEXITED)))
+			wo->notask_error = 0;
+	} else {
+		/*
+		 * @p is alive and it's gonna stop, continue or exit, so
+		 * there always is something to wait for.
+		 */
+		wo->notask_error = 0;
+	}
 
 	if (task_stopped_code(p, ptrace))
 		return wait_task_stopped(wo, ptrace, p);

Currently a real parent can't access job control stopped/continued events through a ptraced child. This utterly breaks job control when the children are ptraced.

For example, if a program is run from an interactive and then strace(1) attaches to it, pressing ^Z would send SIGTSTP and strace(1) would notice it but the shell has no way to tell whether the child entered job control stop and thus can't tell when to take over the terminal - leading to awkward lone ^Z on the terminal.

Because the job control and ptrace stopped states are independent, there is no reason to prevent real parents from accessing the stopped state regardless of ptrace. The continued state isn't separate but ptracers don't have any use for them as ptracees can never resume without explicit command from their ptracers, so as long as ptracers don't consume it, it should be fine.

Although this is a behavior change, because the previous behavior is utterly broken when viewed from real parents and the change is only visible to real parents, I don't think it's necessary to make this behavior optional.

One situation to be careful about is when the real parent is ptracing. The parent is the recipient of both ptrace and job control stop events and one stop can be reported as both job control and ptrace stops. As this can break the current ptrace users, suppress job control stopped events for these cases.

If a real parent ptracer wants to know about both job control and ptrace stops, it can create a separate task to serve the role as real parent.

Note that this only updates wait(2) side of things. The real parent can access the states via wait(2) but still is not properly notified (woken up and delivered signal). Test case polls wait(2) with WNOHANG to work around. Notification will be updated by future patches.

Test case follows.

  #include <stdio.h>
  #include <unistd.h>
  #include <time.h>
  #include <errno.h>
  #include <sys/types.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

int main(void)

  {
	  const struct timespec ts100ms = { .tv_nsec = 100000000 };
	  pid_t tracee, tracer;
	  siginfo_t si;
	  int i;

	  tracee = fork();
	  if (tracee == 0) {
		  while (1) {
			  printf("tracee: SIGSTOP\n");
			  raise(SIGSTOP);
			  nanosleep(&ts100ms, NULL);
			  printf("tracee: SIGCONT\n");
			  raise(SIGCONT);
			  nanosleep(&ts100ms, NULL);
		  }
	  }

	  waitid(P_PID, tracee, &si, WSTOPPED | WNOHANG | WNOWAIT);

	  tracer = fork();
	  if (tracer == 0) {
		  nanosleep(&ts100ms, NULL);
		  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);

		  for (i = 0; i < 11; i++) {
			  si.si_pid = 0;
			  waitid(P_PID, tracee, &si, WSTOPPED);
			  if (si.si_pid && si.si_code == CLD_TRAPPED)
				  ptrace(PTRACE_CONT, tracee, NULL,
					 (void *)(long)si.si_status);
		  }
		  printf("tracer: EXITING\n");
		  return 0;
	  }

	  while (1) {
		  si.si_pid = 0;
		  waitid(P_PID, tracee, &si,
			 WSTOPPED | WCONTINUED | WEXITED | WNOHANG);
		  if (si.si_pid)
			  printf("mommy : WAIT status=%02d code=%02d\n",
				 si.si_status, si.si_code);
		  nanosleep(&ts100ms, NULL);
	  }
	  return 0;

}

Before the patch, while ptraced, the parent can't see any job control events.

tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
tracee: SIGSTOP
tracee: SIGCONT
tracee: SIGSTOP
tracee: SIGCONT
tracee: SIGSTOP
tracer: EXITING
mommy : WAIT status=19 code=05
^C

After the patch,

tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
tracer: EXITING
mommy : WAIT status=19 code=05
^C

---
 kernel/exit.c |   40 ++++++++++++++++++++++++++++++++--------
 1 files changed, 32 insertions(+), 8 deletions(-)

index 7171821..922e41d 100644
@@ -1541,17 +1541,19 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 	if (p->exit_state == EXIT_DEAD)
 		return 0;
 
-	if (likely(!ptrace) && unlikely(task_ptrace(p))) {
+	/* slay zombie? */
+	if (p->exit_state == EXIT_ZOMBIE) {
 		/*
-		 * This child is hidden by ptrace.
-		 * We aren't allowed to see it now, but eventually we will.
+		 * A zombie ptracee is only visible to its ptracer.
+		 * Notification and reaping will be cascaded to the real
+		 * parent when the ptracer detaches.
 		 */
-		wo->notask_error = 0;
-		return 0;
-	}
+		if (likely(!ptrace) && unlikely(task_ptrace(p))) {
+			/* it will become visible, clear notask_error */
+			wo->notask_error = 0;
+			return 0;
+		}
 
-	/* slay zombie? */
-	if (p->exit_state == EXIT_ZOMBIE) {
 		/* we don't reap group leaders with subthreads */
 		if (!delay_group_leader(p))
 			return wait_task_zombie(wo, p);

@@ -1580,15 +1582,37 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
 			wo->notask_error = 0;
 	} else {
 		/*
+		 * If %current is ptracing @p, hide group stop/continued
+		 * state when looking at @p as the real parent; otherwise,
+		 * a single stop can be reported twice as group and ptrace
+		 * stops.
+		 *
+		 * If a ptracer wants to distinguish the two events for its
+		 * own children, it should create a separate task which
+		 * takes the role of real parent.
+		 */
+		if (likely(!ptrace) && task_ptrace(p) && p->parent == current)
+			return 0;
+
+		/*
 		 * @p is alive and it's gonna stop, continue or exit, so
 		 * there always is something to wait for.
 		 */
 		wo->notask_error = 0;
 	}
 
+	/*
+	 * Wait for stopped.  Depending on @ptrace, different stopped state
+	 * is used and the two don't interact with each other.
+	 */
 	if (task_stopped_code(p, ptrace))
 		return wait_task_stopped(wo, ptrace, p);
 
+	/*
+	 * Wait for continued.  There's only one continued state and the
+	 * ptracer can consume it which can confuse the real parent.  Don't
+	 * use WCONTINUED from ptracer.  You don't need or want it.
+	 */
 	return wait_task_continued(wo, p);
 }
 

1.7.1

Yes. This is what we need.

Oh. I'll try to recheck this all once again later, there are too many combinations we should worry about. But looks correct.

Minor problem:

Yes, but

Hello,

Alright, I'll update the test here and in the other patch which has similar logic.

BTW, what are you planning about patch routing? Are you gonna setup a tree?

Thanks.

tejun

Currently a real parent can't access job control stopped/continued events through a ptraced child. This utterly breaks job control when the children are ptraced.

For example, if a program is run from an interactive shell and then strace(1) attaches to it, pressing ^Z would send SIGTSTP and strace(1) would notice it but the shell has no way to tell whether the child entered job control stop and thus can't tell when to take over the terminal - leading to awkward lone ^Z on the terminal.

Because the job control and ptrace stopped states are independent, there is no reason to prevent real parents from accessing the stopped state regardless of ptrace. The continued state isn't separate but ptracers don't have any use for them as ptracees can never resume without explicit command from their ptracers, so as long as ptracers don't consume it, it should be fine.

Although this is a behavior change, because the previous behavior is utterly broken when viewed from real parents and the change is only visible to real parents, I don't think it's necessary to make this behavior optional.

One situation to be careful about is when a task from the real parent's group is ptracing. The parent group is the recipient of both ptrace and job control stop events and one stop can be reported as both job control and ptrace stops. As this can break the current ptrace users, suppress job control stopped events for these cases.

If a real parent ptracer wants to know about both job control and ptrace stops, it can create a separate process to serve the role of real parent.

Note that this only updates wait(2) side of things. The real parent can access the states via wait(2) but still is not properly notified (woken up and delivered signal). Test case polls wait(2) with WNOHANG to work around. Notification will be updated by future patches.

Test case follows.

  #include <stdio.h>
  #include <unistd.h>
  #include <time.h>
  #include <errno.h>
  #include <sys/types.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

int main(void)

  {
	  const struct timespec ts100ms = { .tv_nsec = 100000000 };
	  pid_t tracee, tracer;
	  siginfo_t si;
	  int i;

	  tracee = fork();
	  if (tracee == 0) {
		  while (1) {
			  printf("tracee: SIGSTOP\n");
			  raise(SIGSTOP);
			  nanosleep(&ts100ms, NULL);
			  printf("tracee: SIGCONT\n");
			  raise(SIGCONT);
			  nanosleep(&ts100ms, NULL);
		  }
	  }

	  waitid(P_PID, tracee, &si, WSTOPPED | WNOHANG | WNOWAIT);

	  tracer = fork();
	  if (tracer == 0) {
		  nanosleep(&ts100ms, NULL);
		  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);

		  for (i = 0; i < 11; i++) {
			  si.si_pid = 0;
			  waitid(P_PID, tracee, &si, WSTOPPED);
			  if (si.si_pid && si.si_code == CLD_TRAPPED)
				  ptrace(PTRACE_CONT, tracee, NULL,
					 (void *)(long)si.si_status);
		  }
		  printf("tracer: EXITING\n");
		  return 0;
	  }

	  while (1) {
		  si.si_pid = 0;
		  waitid(P_PID, tracee, &si,
			 WSTOPPED | WCONTINUED | WEXITED | WNOHANG);
		  if (si.si_pid)
			  printf("mommy : WAIT status=%02d code=%02d\n",
				 si.si_status, si.si_code);
		  nanosleep(&ts100ms, NULL);
	  }
	  return 0;

}

Before the patch, while ptraced, the parent can't see any job control events.

tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
tracee: SIGSTOP
tracee: SIGCONT
tracee: SIGSTOP
tracee: SIGCONT
tracee: SIGSTOP
tracer: EXITING
mommy : WAIT status=19 code=05
^C

After the patch,

tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
tracer: EXITING
mommy : WAIT status=19 code=05
^C

-v2: Oleg pointed out that wait(2) should be suppressed for the real
parent's group instead of only the real parent task itself. Updated accordingly.

---
 kernel/exit.c |   41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

Index: work/kernel/exit.c
===================================================================
@@ -1541,17 +1541,19 @@ static int wait_consider_task(struct wai
 	if (p->exit_state == EXIT_DEAD)
 		return 0;
 
-	if (likely(!ptrace) && unlikely(task_ptrace(p))) {
+	/* slay zombie? */
+	if (p->exit_state == EXIT_ZOMBIE) {
 		/*
-		 * This child is hidden by ptrace.
-		 * We aren't allowed to see it now, but eventually we will.
+		 * A zombie ptracee is only visible to its ptracer.
+		 * Notification and reaping will be cascaded to the real
+		 * parent when the ptracer detaches.
 		 */
-		wo->notask_error = 0;
-		return 0;
-	}
+		if (likely(!ptrace) && unlikely(task_ptrace(p))) {
+			/* it will become visible, clear notask_error */
+			wo->notask_error = 0;
+			return 0;
+		}
 
-	/* slay zombie? */
-	if (p->exit_state == EXIT_ZOMBIE) {
 		/* we don't reap group leaders with subthreads */
 		if (!delay_group_leader(p))
 			return wait_task_zombie(wo, p);

@@ -1580,15 +1582,38 @@ static int wait_consider_task(struct wai
 			wo->notask_error = 0;
 	} else {
 		/*
+		 * If @p is ptraced by a task in its real parent's group,
+		 * hide group stop/continued state when looking at @p as
+		 * the real parent; otherwise, a single stop can be
+		 * reported twice as group and ptrace stops.
+		 *
+		 * If a ptracer wants to distinguish the two events for its
+		 * own children, it should create a separate process which
+		 * takes the role of real parent.
+		 */
+		if (likely(!ptrace) && task_ptrace(p) &&
+		    same_thread_group(p->parent, p->real_parent))
+			return 0;
+
+		/*
 		 * @p is alive and it's gonna stop, continue or exit, so
 		 * there always is something to wait for.
 		 */
 		wo->notask_error = 0;
 	}
 
+	/*
+	 * Wait for stopped.  Depending on @ptrace, different stopped state
+	 * is used and the two don't interact with each other.
+	 */
 	if (task_stopped_code(p, ptrace))
 		return wait_task_stopped(wo, ptrace, p);
 
+	/*
+	 * Wait for continued.  There's only one continued state and the
+	 * ptracer can consume it which can confuse the real parent.  Don't
+	 * use WCONTINUED from ptracer.  You don't need or want it.
+	 */
 	return wait_task_continued(wo, p);
 }
 

Currently, do_notify_parent_cldstop() determines whether the notification is for the real parent or ptracer. Move the decision to the caller by adding @for_ptrace parameter to do_notify_parent_cldstop(). All the callers are updated to pass task_ptrace(target_task), so this patch doesn't cause any behavior difference.

While at it, add function comment to do_notify_parent_cldstop().

---
 kernel/signal.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

index 6130cb8..4acca00 100644
@@ -1595,16 +1595,30 @@ int do_notify_parent(struct task_struct *tsk, int sig)
 	return ret;
 }
 
-static void do_notify_parent_cldstop(struct task_struct *tsk, int why)
+/**
+ * do_notify_parent_cldstop - notify parent of stopped/continued state change
+ * @tsk: task reporting the state change
+ * @for_ptracer: the notification is for ptracer
+ * @why: CLD_{CONTINUED|STOPPED|TRAPPED} to report
+ *
+ * Notify @tsk's parent that the stopped/continued state has changed.  If
+ * @for_ptracer is %false, @tsk's group leader notifies to its real parent.
+ * If %true, @tsk reports to @tsk->parent which should be the ptracer.
+ *
+ * CONTEXT:
+ * Must be called with tasklist_lock at least read locked.
+ */
+static void do_notify_parent_cldstop(struct task_struct *tsk,
+				     bool for_ptracer, int why)
 {
 	struct siginfo info;
 	unsigned long flags;
 	struct task_struct *parent;
 	struct sighand_struct *sighand;
 
-	if (task_ptrace(tsk))
+	if (for_ptracer) {
 		parent = tsk->parent;
-	else {
+	} else {
 		tsk = tsk->group_leader;
 		parent = tsk->real_parent;
 	}

@@ -1743,7 +1757,7 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info)
 	spin_unlock_irq(&current->sighand->siglock);
 	read_lock(&tasklist_lock);
 	if (may_ptrace_stop()) {
-		do_notify_parent_cldstop(current, why);
+		do_notify_parent_cldstop(current, task_ptrace(current), why);
 		/*
 		 * Don't want to allow preemption here, because
 		 * sys_ptrace() needs this task to be inactive.

@@ -1886,7 +1900,8 @@ retry:
 
 		if (notify) {
 			read_lock(&tasklist_lock);
-			do_notify_parent_cldstop(current, notify);
+			do_notify_parent_cldstop(current, task_ptrace(current),
+						 notify);
 			read_unlock(&tasklist_lock);
 		}
 

@@ -1982,6 +1997,7 @@ relock:
 	 * the CLD_ si_code into SIGNAL_CLD_MASK bits.
 	 */
 	if (unlikely(signal->flags & SIGNAL_CLD_MASK)) {
+		struct task_struct *leader;
 		int why;
 
 		if (signal->flags & SIGNAL_CLD_CONTINUED)

@@ -1994,7 +2010,8 @@ relock:
 		spin_unlock_irq(&sighand->siglock);
 
 		read_lock(&tasklist_lock);
-		do_notify_parent_cldstop(current->group_leader, why);
+		leader = current->group_leader;
+		do_notify_parent_cldstop(leader, task_ptrace(leader), why);
 		read_unlock(&tasklist_lock);
 		goto relock;
 	}

@@ -2167,7 +2184,7 @@ out:
 
 	if (unlikely(group_stop)) {
 		read_lock(&tasklist_lock);
-		do_notify_parent_cldstop(tsk, group_stop);
+		do_notify_parent_cldstop(tsk, task_ptrace(tsk), group_stop);
 		read_unlock(&tasklist_lock);
 	}
 }

1.7.1

The stopped notifications in do_signal_stop() and exit_signals() are always for the completion of job control. The one in do_signal_stop() may be delivered to the ptracer if PTRACE_ATTACH races with notification and the one in exit_signals() if task exits while ptraced.

In both cases, the notifications are meaningless and confusing to the ptracer as it never accesses the group stop state while the real parent would miss notifications for the events it is watching.

Make sure these notifications always go to the real parent by calling do_notify_parent_cld_stop() with %false @for_ptrace.

---
 kernel/signal.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

index 4acca00..52120d6 100644
@@ -1898,10 +1898,18 @@ retry:
 		__set_current_state(TASK_STOPPED);
 		spin_unlock_irq(&current->sighand->siglock);
 
+		/*
+		 * Notify the parent of the group stop completion.  Because
+		 * we're not holding either the siglock or tasklist_lock
+		 * here, ptracer may attach inbetween; however, this is for
+		 * group stop and should always be delivered to the real
+		 * parent of the group leader.  The new ptracer will get
+		 * its notification when this task transitions into
+		 * TASK_TRACED.
+		 */
 		if (notify) {
 			read_lock(&tasklist_lock);
-			do_notify_parent_cldstop(current, task_ptrace(current),
-						 notify);
+			do_notify_parent_cldstop(current, false, notify);
 			read_unlock(&tasklist_lock);
 		}
 

@@ -2182,9 +2190,13 @@ void exit_signals(struct task_struct *tsk)
 out:
 	spin_unlock_irq(&tsk->sighand->siglock);
 
+	/*
+	 * If group stop has completed, deliver the notification.  This
+	 * should always go to the real parent of the group leader.
+	 */
 	if (unlikely(group_stop)) {
 		read_lock(&tasklist_lock);
-		do_notify_parent_cldstop(tsk, task_ptrace(tsk), group_stop);
+		do_notify_parent_cldstop(tsk, false, group_stop);
 		read_unlock(&tasklist_lock);
 	}
 }

1.7.1

Yes, looks correct.

Oleg.

With recent changes, job control and ptrace stopped states are properly separated and accessible to the real parent and the ptracer respectively; however, notifications of job control stopped/continued events to the real parent while ptraced are still missing.

A ptracee participates in group stop in ptrace_stop() but the completion isn't notified. If participation results in completion of group stop, notify the real parent of the event. The ptrace and group stops are separate and can be handled as such.

However, when the real parent is the ptracer, only the ptrace stop event is visible through wait(2) and the duplicate notifications are different from the current behaivor and are confusing. Suppress group stop notification in such cases.

The continued state is shared between the real parent and the ptracer but is only meaningful to the real parent. Always notify the real parent and notify the ptracer too for backward compatibility. Similar to stop notification, if the real parent is the ptracer, suppress a duplicate notification.

Test case follows.

  #include <stdio.h>
  #include <unistd.h>
  #include <time.h>
  #include <errno.h>
  #include <sys/types.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

int main(void)

  {
	  const struct timespec ts100ms = { .tv_nsec = 100000000 };
	  pid_t tracee, tracer;
	  siginfo_t si;
	  int i;

	  tracee = fork();
	  if (tracee == 0) {
		  while (1) {
			  printf("tracee: SIGSTOP\n");
			  raise(SIGSTOP);
			  nanosleep(&ts100ms, NULL);
			  printf("tracee: SIGCONT\n");
			  raise(SIGCONT);
			  nanosleep(&ts100ms, NULL);
		  }
	  }

	  waitid(P_PID, tracee, &si, WSTOPPED | WNOHANG | WNOWAIT);

	  tracer = fork();
	  if (tracer == 0) {
		  nanosleep(&ts100ms, NULL);
		  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);

		  for (i = 0; i < 11; i++) {
			  si.si_pid = 0;
			  waitid(P_PID, tracee, &si, WSTOPPED);
			  if (si.si_pid && si.si_code == CLD_TRAPPED)
				  ptrace(PTRACE_CONT, tracee, NULL,
					 (void *)(long)si.si_status);
		  }
		  printf("tracer: EXITING\n");
		  return 0;
	  }

	  while (1) {
		  si.si_pid = 0;
		  waitid(P_PID, tracee, &si, WSTOPPED | WCONTINUED | WEXITED);
		  if (si.si_pid)
			  printf("mommy : WAIT status=%02d code=%02d\n",
				 si.si_status, si.si_code);
	  }
	  return 0;

}

Before this patch, while ptraced, the real parent doesn't get notifications for job control events, so although it can access those events, the later waitid(2) call never wakes up.

tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
tracee: SIGSTOP
tracee: SIGCONT
tracee: SIGSTOP
tracee: SIGCONT
tracee: SIGSTOP
tracer: EXITING
mommy : WAIT status=19 code=05
^C

After this patch, it works as expected.

tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
tracer: EXITING
mommy : WAIT status=19 code=05
^C

---
 kernel/signal.c |   44 +++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 41 insertions(+), 3 deletions(-)

index 52120d6..74f097c 100644
@@ -1694,6 +1694,15 @@ static int sigkill_pending(struct task_struct *tsk)
 }
 
 /*
+ * Test whether the target task of the usual cldstop notification - the
+ * real_parent of the group_leader of @child - is the ptracer.
+ */
+static bool real_parent_is_ptracer(struct task_struct *child)
+{
+	return child->parent == child->group_leader->real_parent;
+}
+
+/*
  * This must be called with current->sighand->siglock held.
  *
  * This should be the path for all ptrace stops.

@@ -1708,6 +1717,8 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info)
 	__releases(&current->sighand->siglock)
 	__acquires(&current->sighand->siglock)
 {
+	bool gstop_done = false;
+
 	if (arch_ptrace_stop_needed(exit_code, info)) {
 		/*
 		 * The arch code has something special to do before a

@@ -1735,7 +1746,7 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info)
 	 * is entered - ignore it.
 	 */
 	if (why == CLD_STOPPED && (current->group_stop & GROUP_STOP_PENDING))
-		task_participate_group_stop(current);
+		gstop_done = task_participate_group_stop(current);
 
 	current->last_siginfo = info;
 	current->exit_code = exit_code;

@@ -1757,7 +1768,20 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info)
 	spin_unlock_irq(&current->sighand->siglock);
 	read_lock(&tasklist_lock);
 	if (may_ptrace_stop()) {
-		do_notify_parent_cldstop(current, task_ptrace(current), why);
+		/*
+		 * Notify parents of the stop.
+		 *
+		 * While ptraced, there are two parents - the ptracer and
+		 * the real_parent of the group_leader.  The ptracer should
+		 * know about every stop while the real parent is only
+		 * interested in the completion of group stop.  The states
+		 * for the two don't interact with each other.  Notify
+		 * separately unless they're gonna be duplicates.
+		 */
+		do_notify_parent_cldstop(current, true, why);
+		if (gstop_done && !real_parent_is_ptracer(current))
+			do_notify_parent_cldstop(current, false, why);
+
 		/*
 		 * Don't want to allow preemption here, because
 		 * sys_ptrace() needs this task to be inactive.

@@ -2017,10 +2041,24 @@ relock:
 
 		spin_unlock_irq(&sighand->siglock);
 
+		/*
+		 * Notify the parent that we're continuing.  This event is
+		 * always per-process and doesn't make whole lot of sense
+		 * for ptracers, who shouldn't consume the state via
+		 * wait(2) either, but, for backward compatibility, notify
+		 * the ptracer of the group leader too unless it's gonna be
+		 * a duplicate.
+		 */
 		read_lock(&tasklist_lock);
+
+		do_notify_parent_cldstop(current, false, why);
+
 		leader = current->group_leader;
-		do_notify_parent_cldstop(leader, task_ptrace(leader), why);
+		if (task_ptrace(leader) && !real_parent_is_ptracer(leader))
+			do_notify_parent_cldstop(leader, true, why);
+
 		read_unlock(&tasklist_lock);
+
 		goto relock;
 	}
 

1.7.1

Yes, great.

Yeah, same_thread_group() would be better.

Care to elaborate?

You mean when may_ptrace_stop() fails, right? Yeah, we need notification in the else clause. Will add it.

Thanks.

tejun

child->real_parent must be equal to child->group_leader->real_parent.

What we need is ptrace_reparented(). But as I said, it should be fixed first (I think) and this needs a separate patch/discussion.

So yes, we need a new helper.

Well, and look at get_signal_to_deliver(),

		leader = current->group_leader;
		if (task_ptrace(leader) && !real_parent_is_ptracer(leader))
			do_notify_parent_cldstop(leader, true, why);

Indeed. But in fact task_ptrace(leader) is not needed. If real_parent_is_ptracer(tsk) returns false, then tsk must be ptraced.

So we could write

		if (!real_parent_is_ptracer(leader))
			do_notify_parent_cldstop(leader, true, why);

but now this looks confusing because of the naming. Compare with

		if (ptrace_reparented(leader))
			do_notify_parent_cldstop(leader);

Oleg.

Hey, Oleg.

Okay.

I see but I don't think ptrace_reparented() is a good name for !same_thread_group(child->parent, child->real_parent). It gets too confusing with child->parent != child->real_parent test.

Anyways, I'll leave the renaming for later patch and just update the ->group_leader part.

Thanks.

tejun

Ooh, on a related note, we probably want to change do_notify_parent_cldstop() too. tsk->group_leader->real_parent is used as the delivery target when !@for_ptracer. This is the same with tsk->real_parent and the code has been like this for a long time but is a bit confusing.

Thanks.

tejun

Yes, although in this case we do

		tsk = tsk->group_leader;
		parent = tsk->real_parent;

We need to change tsk to report the correct si_pid. But we could do

		parent = tsk->real_parent;
		tsk = tsk->group_leader;

not sure this looks less confusing.

Oleg.

Hello,

They themselves are about the same but the inconsistency with the is_real_parent() test is a bit confusing, I think. If we're testing for duplicates by testing ->real_parent against ->parent then the actual delivery should be also be using ->real_parent and ->parent, so...

Thanks.

tejun

Hmm. So I misunderstood you. And still can't understand...

Could you explain how should we change do_notify_parent_cldstop()? I am just curious.

Oleg.

Hello,

As you suggested above. :-) The inconsistency I'm concernced about is the following.

/*
* Test whether the target task of the usual cldstop notification - the
* real_parent of @child - is in the same group as the ptracer. */
static bool real_parent_is_ptracer(struct task_struct *child)

 {
	 return same_thread_group(child->parent, child->real_parent);

}

So, here, we're saying that if child->parent and child->real_parent are in the same thread group, they're gonna be duplicates, but in do_notify_parent_cldstop(), we target tsk->group_leader->real_parent, which should be the same as tsk->real_parent but confusing nonetheless. That's actually the reason why I used child->group_leader->real_parent in the first place.

Thanks.

tejun

Ah! got it. I misread you email as if I missed your initial point, sorry.

Oleg.

With recent changes, job control and ptrace stopped states are properly separated and accessible to the real parent and the ptracer respectively; however, notifications of job control stopped/continued events to the real parent while ptraced are still missing.

A ptracee participates in group stop in ptrace_stop() but the completion isn't notified. If participation results in completion of group stop, notify the real parent of the event. The ptrace and group stops are separate and can be handled as such.

However, when the real parent and the ptracer are in the same thread group, only the ptrace stop event is visible through wait(2) and the duplicate notifications are different from the current behavior and are confusing. Suppress group stop notification in such cases.

The continued state is shared between the real parent and the ptracer but is only meaningful to the real parent. Always notify the real parent and notify the ptracer too for backward compatibility. Similar to stop notification, if the real parent is the ptracer, suppress a duplicate notification.

Test case follows.

  #include <stdio.h>
  #include <unistd.h>
  #include <time.h>
  #include <errno.h>
  #include <sys/types.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

int main(void)

  {
	  const struct timespec ts100ms = { .tv_nsec = 100000000 };
	  pid_t tracee, tracer;
	  siginfo_t si;
	  int i;

	  tracee = fork();
	  if (tracee == 0) {
		  while (1) {
			  printf("tracee: SIGSTOP\n");
			  raise(SIGSTOP);
			  nanosleep(&ts100ms, NULL);
			  printf("tracee: SIGCONT\n");
			  raise(SIGCONT);
			  nanosleep(&ts100ms, NULL);
		  }
	  }

	  waitid(P_PID, tracee, &si, WSTOPPED | WNOHANG | WNOWAIT);

	  tracer = fork();
	  if (tracer == 0) {
		  nanosleep(&ts100ms, NULL);
		  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);

		  for (i = 0; i < 11; i++) {
			  si.si_pid = 0;
			  waitid(P_PID, tracee, &si, WSTOPPED);
			  if (si.si_pid && si.si_code == CLD_TRAPPED)
				  ptrace(PTRACE_CONT, tracee, NULL,
					 (void *)(long)si.si_status);
		  }
		  printf("tracer: EXITING\n");
		  return 0;
	  }

	  while (1) {
		  si.si_pid = 0;
		  waitid(P_PID, tracee, &si, WSTOPPED | WCONTINUED | WEXITED);
		  if (si.si_pid)
			  printf("mommy : WAIT status=%02d code=%02d\n",
				 si.si_status, si.si_code);
	  }
	  return 0;

}

Before this patch, while ptraced, the real parent doesn't get notifications for job control events, so although it can access those events, the later waitid(2) call never wakes up.

tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
tracee: SIGSTOP
tracee: SIGCONT
tracee: SIGSTOP
tracee: SIGCONT
tracee: SIGSTOP
tracer: EXITING
mommy : WAIT status=19 code=05
^C

After this patch, it works as expected.

tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
mommy : WAIT status=19 code=05
tracee: SIGCONT
mommy : WAIT status=18 code=06
tracee: SIGSTOP
tracer: EXITING
mommy : WAIT status=19 code=05
^C

-v2: Oleg pointed out that

* Group stop notification to the real parent should also happen when ptracer detach races with ptrace_stop().

* real_parent_is_ptracer() should be testing thread group equality not the task itself as wait(2) and stop/cont notifications are normally thread-group wide.

Both issues are fixed accordingly.

---
 kernel/signal.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 3 deletions(-)

Index: work/kernel/signal.c
===================================================================
@@ -1694,6 +1694,17 @@ static int sigkill_pending(struct task_s
 }
 
 /*
+ * Test whether the target task of the usual cldstop notification - the
+ * real_parent of the group_leader of @child - is in the same group as the
+ * ptracer.
+ */
+static bool real_parent_is_ptracer(struct task_struct *child)
+{
+	return same_thread_group(child->parent,
+				 child->group_leader->real_parent);
+}
+
+/*
  * This must be called with current->sighand->siglock held.
  *
  * This should be the path for all ptrace stops.

@@ -1708,6 +1719,8 @@ static void ptrace_stop(int exit_code, i
 	__releases(&current->sighand->siglock)
 	__acquires(&current->sighand->siglock)
 {
+	bool gstop_done = false;
+
 	if (arch_ptrace_stop_needed(exit_code, info)) {
 		/*
 		 * The arch code has something special to do before a

@@ -1735,7 +1748,7 @@ static void ptrace_stop(int exit_code, i
 	 * is entered - ignore it.
 	 */
 	if (why == CLD_STOPPED && (current->group_stop & GROUP_STOP_PENDING))
-		task_participate_group_stop(current);
+		gstop_done = task_participate_group_stop(current);
 
 	current->last_siginfo = info;
 	current->exit_code = exit_code;

@@ -1757,7 +1770,20 @@ static void ptrace_stop(int exit_code, i
 	spin_unlock_irq(&current->sighand->siglock);
 	read_lock(&tasklist_lock);
 	if (may_ptrace_stop()) {
-		do_notify_parent_cldstop(current, task_ptrace(current), why);
+		/*
+		 * Notify parents of the stop.
+		 *
+		 * While ptraced, there are two parents - the ptracer and
+		 * the real_parent of the group_leader.  The ptracer should
+		 * know about every stop while the real parent is only
+		 * interested in the completion of group stop.  The states
+		 * for the two don't interact with each other.  Notify
+		 * separately unless they're gonna be duplicates.
+		 */
+		do_notify_parent_cldstop(current, true, why);
+		if (gstop_done && !real_parent_is_ptracer(current))
+			do_notify_parent_cldstop(current, false, why);
+
 		/*
 		 * Don't want to allow preemption here, because
 		 * sys_ptrace() needs this task to be inactive.

@@ -1772,7 +1798,16 @@ static void ptrace_stop(int exit_code, i
 		/*
 		 * By the time we got the lock, our tracer went away.
 		 * Don't drop the lock yet, another tracer may come.
+		 *
+		 * If @gstop_done, the ptracer went away between group stop
+		 * completion and here.  During detach, it would have set
+		 * GROUP_STOP_PENDING on us and we'll re-enter TASK_STOPPED
+		 * in do_signal_stop() on return, so notifying the real
+		 * parent of the group stop completion is enough.
 		 */
+		if (gstop_done)
+			do_notify_parent_cldstop(current, false, why);
+
 		__set_current_state(TASK_RUNNING);
 		if (clear_code)
 			current->exit_code = 0;

@@ -2017,10 +2052,24 @@ relock:
 
 		spin_unlock_irq(&sighand->siglock);
 
+		/*
+		 * Notify the parent that we're continuing.  This event is
+		 * always per-process and doesn't make whole lot of sense
+		 * for ptracers, who shouldn't consume the state via
+		 * wait(2) either, but, for backward compatibility, notify
+		 * the ptracer of the group leader too unless it's gonna be
+		 * a duplicate.
+		 */
 		read_lock(&tasklist_lock);
+
+		do_notify_parent_cldstop(current, false, why);
+
 		leader = current->group_leader;
-		do_notify_parent_cldstop(leader, task_ptrace(leader), why);
+		if (task_ptrace(leader) && !real_parent_is_ptracer(leader))
+			do_notify_parent_cldstop(leader, true, why);
+
 		read_unlock(&tasklist_lock);
+
 		goto relock;
 	}
 

Ok, from skimming the patches, I like the whole series. I especially liked how all the steps were small and well-commented, both in the code and in the changelog. I could basically follow the patch series from just reading the changelogs, and the patches themselves looked fine too.

Linus

Just as group_exit_code shouldn't be generated when a PTRACE_CONT'd task re-enters job control stop, notifiction for the event should be suppressed too. The logic is the same as the group_exit_code generation suppression in do_signal_stop(), if SIGNAL_STOP_STOPPED is already set, the task is re-entering job control stop without intervening SIGCONT and the notifications should be suppressed.

Test case follows.

  #include <stdio.h>
  #include <unistd.h>
  #include <signal.h>
  #include <time.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

  static const struct timespec ts100ms = { .tv_nsec = 100000000 };
  static pid_t tracee, tracer;

static const char *pid_who(pid_t pid)

  {
	  return pid == tracee ? "tracee" : (pid == tracer ? "tracer" : "mommy ");

}

static void sigchld_sigaction(int signo, siginfo_t *si, void *ucxt)

  {
	  printf("%s: SIG status=%02d code=%02d (%s)\n",
		 pid_who(getpid()), si->si_status, si->si_code,
		 pid_who(si->si_pid));

}

int main(void)

  {
	  const struct sigaction chld_sa = { .sa_sigaction = sigchld_sigaction,
					     .sa_flags = SA_SIGINFO|SA_RESTART };
	  siginfo_t si;

	  sigaction(SIGCHLD, &chld_sa, NULL);

	  tracee = fork();
	  if (!tracee) {
		  tracee = getpid();
		  while (1)
			  pause();
	  }

	  kill(tracee, SIGSTOP);
	  waitid(P_PID, tracee, &si, WSTOPPED);

	  tracer = fork();
	  if (!tracer) {
		  tracer = getpid();
		  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);
		  waitid(P_PID, tracee, &si, WSTOPPED);
		  ptrace(PTRACE_CONT, tracee, NULL, (void *)(long)si.si_status);
		  waitid(P_PID, tracee, &si, WSTOPPED);
		  ptrace(PTRACE_CONT, tracee, NULL, (void *)(long)si.si_status);
		  waitid(P_PID, tracee, &si, WSTOPPED);
		  printf("tracer: detaching\n");
		  ptrace(PTRACE_DETACH, tracee, NULL, NULL);
		  return 0;
	  }

	  while (1)
		  pause();
	  return 0;

}

Before the patch, the parent gets the second notification for the tracee after the tracer detaches. si_status is zero because group_exit_code is not set by the group stop completion which triggered this notification.

mommy : SIG status=19 code=05 (tracee)
tracer: SIG status=00 code=05 (tracee)
tracer: SIG status=19 code=04 (tracee)
tracer: SIG status=00 code=05 (tracee)
tracer: detaching
mommy : SIG status=00 code=05 (tracee)
mommy : SIG status=00 code=01 (tracer)
^C

After the patch, the duplicate notification is gone.

mommy : SIG status=19 code=05 (tracee)
tracer: SIG status=00 code=05 (tracee)
tracer: SIG status=19 code=04 (tracee)
tracer: SIG status=00 code=05 (tracee)
tracer: detaching
mommy : SIG status=00 code=01 (tracer)
^C

---
 kernel/signal.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

index 74f097c..8304563 100644
@@ -268,6 +268,10 @@ void task_clear_group_stop_pending(struct task_struct *task)
  *
  * CONTEXT:
  * Must be called with @task->sighand->siglock held.
+ *
+ * RETURNS:
+ * %true if group stop completion should be notified to the parent, %false
+ * otherwise.
  */
 static bool task_participate_group_stop(struct task_struct *task)
 {

@@ -284,7 +288,11 @@ static bool task_participate_group_stop(struct task_struct *task)
 	if (!WARN_ON_ONCE(sig->group_stop_count == 0))
 		sig->group_stop_count--;
 
-	if (!sig->group_stop_count) {
+	/*
+	 * Tell the caller to notify completion iff we are entering into a
+	 * fresh group stop.  Read comment in do_signal_stop() for details.
+	 */
+	if (!sig->group_stop_count && !(sig->flags & SIGNAL_STOP_STOPPED)) {
 		sig->flags = SIGNAL_STOP_STOPPED;
 		return true;
 	}

1.7.1

And at first glance this does what we discussed before. I mean, even if (perhaps) this series has some problems/bugs it looks right "in general" to me.

Yeah... And right now I have some concerns about the details, but I am not ready to share them. Will try tomorrow, it turns out I need to re-read the previous series.

Say, even 1/8 doesn't look exactly right with the current ptrace code, WARN_ON_ONCE() can be easily triggered afaics.

Oleg.

Remove the extra task_is_traced() check in __ptrace_unlink() and collapse ptrace_untrace() into __ptrace_unlink(). This is to prepare for further changes.

While at it, drop the comment on top of ptrace_untrace() and convert __ptrace_unlink() comment to docbook format. Detailed comment will be added by the next patch.

This patch doesn't cause any visible behavior changes.

---
 kernel/ptrace.c |   40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

Index: work/kernel/ptrace.c
===================================================================
@@ -37,15 +37,23 @@ void __ptrace_link(struct task_struct *c
 	child->parent = new_parent;
 }
 
-/*
- * Turn a tracing stop into a normal stop now, since with no tracer there
- * would be no way to wake it up with SIGCONT or SIGKILL.  If there was a
- * signal sent that would resume the child, but didn't because it was in
- * TASK_TRACED, resume it now.
- * Requires that irqs be disabled.
+/**
+ * __ptrace_unlink - unlink ptracee and restore its execution state
+ * @child: ptracee to be unlinked
+ *
+ * Remove @child from the ptrace list, move it back to the original parent.
+ *
+ * CONTEXT:
+ * write_lock_irq(tasklist_lock)
  */
-static void ptrace_untrace(struct task_struct *child)
+void __ptrace_unlink(struct task_struct *child)
 {
+	BUG_ON(!child->ptrace);
+
+	child->ptrace = 0;
+	child->parent = child->real_parent;
+	list_del_init(&child->ptrace_entry);
+
 	spin_lock(&child->sighand->siglock);
 	if (task_is_traced(child)) {
 		/*

@@ -70,24 +78,6 @@ static void ptrace_untrace(struct task_s
 }
 
 /*
- * unptrace a task: move it back to its original parent and
- * remove it from the ptrace list.
- *
- * Must be called with the tasklist lock write-held.
- */
-void __ptrace_unlink(struct task_struct *child)
-{
-	BUG_ON(!child->ptrace);
-
-	child->ptrace = 0;
-	child->parent = child->real_parent;
-	list_del_init(&child->ptrace_entry);
-
-	if (task_is_traced(child))
-		ptrace_untrace(child);
-}
-
-/*
  * Check that we have indeed attached to the thing..
  */
 int ptrace_check_attach(struct task_struct *child, int kill)

Currently, __ptrace_unlink() wakes up the tracee iff it's in TASK_TRACED. For unlinking from PTRACE_DETACH, this is correct as the tracee is guaranteed to be in TASK_TRACED or dead; however, unlinking also happens when the ptracer exits and in this case the ptracee can be in any state and ptrace might be left running even if the group it belongs to is stopped.

This patch updates __ptrace_unlink() such that GROUP_STOP_PENDING is reinstated regardless of the ptracee's current state as long as it's alive and makes sure that signal_wake_up() is called if execution state transition is necessary.

Test case follows.

  #include <unistd.h>
  #include <time.h>
  #include <sys/types.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

  static const struct timespec ts1s = { .tv_sec = 1 };

int main(void)

  {
	  pid_t tracee;
	  siginfo_t si;

	  tracee = fork();
	  if (tracee == 0) {
		  while (1) {
			  nanosleep(&ts1s, NULL);
			  write(1, ".", 1);
		  }
	  }

	  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);
	  waitid(P_PID, tracee, &si, WSTOPPED);
	  ptrace(PTRACE_CONT, tracee, NULL, (void *)(long)si.si_status);
	  waitid(P_PID, tracee, &si, WSTOPPED);
	  ptrace(PTRACE_CONT, tracee, NULL, (void *)(long)si.si_status);
	  write(1, "exiting", 7);
	  return 0;

}

Before the patch, after the parent process exits, the child is left running and prints out "." every second.

exiting..... (continues)

After the patch, the group stop initiated by the implied SIGSTOP from PTRACE_ATTACH is re-established when the parent exits.

exiting

---
 kernel/ptrace.c |   59 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 20 deletions(-)

Index: work/kernel/ptrace.c
===================================================================
@@ -41,7 +41,26 @@ void __ptrace_link(struct task_struct *c
  * __ptrace_unlink - unlink ptracee and restore its execution state
  * @child: ptracee to be unlinked
  *
- * Remove @child from the ptrace list, move it back to the original parent.
+ * Remove @child from the ptrace list, move it back to the original parent,
+ * and restore the execution state so that it conforms to the group stop
+ * state.
+ *
+ * Unlinking can happen via two paths - explicit PTRACE_DETACH or ptracer
+ * exiting.  For PTRACE_DETACH, unless the ptracee has been killed between
+ * ptrace_check_attach() and here, it's guaranteed to be in TASK_TRACED.
+ * If the ptracer is exiting, the ptracee can be in any state.
+ *
+ * After detach, the ptracee should be in a state which conforms to the
+ * group stop.  If the group is stopped or in the process of stopping, the
+ * ptracee should be put into TASK_STOPPED; otherwise, it should be woken
+ * up from TASK_TRACED.
+ *
+ * If the ptracee is in TASK_TRACED and needs to be moved to TASK_STOPPED,
+ * it goes through TRACED -> RUNNING -> STOPPED transition which is similar
+ * to but in the opposite direction of what happens while attaching to a
+ * stopped task.  However, in this direction, the intermediate RUNNING
+ * state is not hidden even from the current ptracer and if it immediately
+ * re-attaches and performs a WNOHANG wait(2), it may fail.
  *
  * CONTEXT:
  * write_lock_irq(tasklist_lock)

@@ -55,25 +74,25 @@ void __ptrace_unlink(struct task_struct
 	list_del_init(&child->ptrace_entry);
 
 	spin_lock(&child->sighand->siglock);
-	if (task_is_traced(child)) {
-		/*
-		 * If group stop is completed or in progress, it should
-		 * participate in the group stop.  Set GROUP_STOP_PENDING
-		 * before kicking it.
-		 *
-		 * This involves TRACED -> RUNNING -> STOPPED transition
-		 * which is similar to but in the opposite direction of
-		 * what happens while attaching to a stopped task.
-		 * However, in this direction, the intermediate RUNNING
-		 * state is not hidden even from the current ptracer and if
-		 * it immediately re-attaches and performs a WNOHANG
-		 * wait(2), it may fail.
-		 */
-		if (child->signal->flags & SIGNAL_STOP_STOPPED ||
-		    child->signal->group_stop_count)
-			child->group_stop |= GROUP_STOP_PENDING;
-		signal_wake_up(child, 1);
-	}
+
+	/*
+	 * Reinstate GROUP_STOP_PENDING if group stop is in effect and
+	 * @child isn't dead.
+	 */
+	if (!(child->flags & PF_EXITING) &&
+	    (child->signal->flags & SIGNAL_STOP_STOPPED ||
+	     child->signal->group_stop_count))
+		child->group_stop |= GROUP_STOP_PENDING;
+
+	/*
+	 * If transition to TASK_STOPPED is pending or in TASK_TRACED, kick
+	 * @child in the butt.  Note that @resume should be used iff @child
+	 * is in TASK_TRACED; otherwise, we might unduly disrupt
+	 * TASK_KILLABLE sleeps.
+	 */
+	if (child->group_stop & GROUP_STOP_PENDING || task_is_traced(child))
+		signal_wake_up(child, task_is_traced(child));
+
 	spin_unlock(&child->sighand->siglock);
 }
 

Looks correct (and the previous one too).

But I don't understand the PF_EXITING check,

	if (SIGNAL_STOP_STOPPED || group_stop_count) {
		child->group_stop |= GROUP_STOP_PENDING;
		signal_wake_up(child, 0);
	}

	if (task_is_traced(child))
		wake_up_state(TASK_TRACED);

Hello,

I don't think it's strictly necessary either but in the usual group stop initiation path in do_signal_stop(), PF_EXITING is checked before setting PENDING, so I think it's better to remain consistent with that.

Yeah, I'd like to avoid mixing calls to wake_up_state() and signal_wake_up(). It gets confusing and there's no reason for micro-optimization here.

Thanks.

tejun

Hello, Oleg.

The updated patches should address the issues pointed out in your reviews, at least the ones which are appropriate to address now. Please let me know when you want the whole series resent.

Thanks.

tejun

Yes, thanks. "UPDATED" patches look fine. Please feel free to add my reviewed-by to the whole series.

I'd only ask you to remove ->group_leader from real_parent_is_ptracer() (btw, note that wait_consider_task() can use this helper too) although this is minor.

Cough. Resent? If you ask me, I think you should just send them to Linus while the merge window is opened.

I am trying to get the account on kernel.org, but until then I hope you can flush the pending ptrace-review branches.

In case I misunderstood you... I think that the whole series is good and the patches should not be updated. Just for example, I'd like to rename real_parent_is_ptracer (and remove ->group_leader), but we can make another patch for that.

And yes, I bet we missed something and we will need the subsequent fixes anyway ;)

Oleg.

Hello,

Yeah, will also rename the function as suggested.

Too late for that. Let's target the next merge window.

I'll set up a branch until you can get your tree up and request inclusion to linux-next.

Oh yeah, I have no doubt about that.

Thanks.

tejun