ptrace,signal: group stop / ptrace updates

NAK. Let's have the wake_up_state(task, TASK_TRACED|TASK_STOPPED) version go in first. That one will be more appropriate for -stable. Even if we shortly remove it entirely in mainline, having the more conservative intermediate state in the git history will make any future problems more susceptible to bisection.

kernel/ptrace.c int ptrace_detach(struct task_struct *ch · line 316

Quoted 9 lines · Tejun Heo

kernel/ptrace.c +1 -1

@@ -313,7 +313,7 @@ int ptrace_detach(struct task_struct *ch
 		child->exit_code = data;
 		dead = __ptrace_detach(current, child);
 		if (!child->exit_state)
-			wake_up_process(child);
+			wake_up_state(child, TASK_TRACED | TASK_STOPPED);

Well, it can't be TASK_TRACED at this point. And of course this still contradicts to __set_task_state(child, TASK_STOPPED) in ptrace_untrace(). IOW, to me the previous patch makes more sense.

But OK, I understand Roland's concerns. And, at least this change fixes the bug mentioned in 95a3540d.

Acked-by Oleg Nesterov <[email protected]>

Hello,

Quoted 14 lines · Oleg Nesterov

kernel/ptrace.c int ptrace_detach(struct task_struct *ch · line 316

Quoted 9 lines · Tejun Heo

kernel/ptrace.c +1 -1

@@ -313,7 +313,7 @@ int ptrace_detach(struct task_struct *ch
 		child->exit_code = data;
 		dead = __ptrace_detach(current, child);
 		if (!child->exit_state)
-			wake_up_process(child);
+			wake_up_state(child, TASK_TRACED | TASK_STOPPED);

Well, it can't be TASK_TRACED at this point. And of course this still contradicts to __set_task_state(child, TASK_STOPPED) in ptrace_untrace(). IOW, to me the previous patch makes more sense.

Yeah, it can't be in TRACED but the whole point of the patch is avoiding rude wakeups, so as long as it doesn't end up waking random [un]interruptible sleeps... It will be removed later anyway.

Quoted 4 lines

But OK, I understand Roland's concerns. And, at least this change fixes the bug mentioned in 95a3540d.

Acked-by Oleg Nesterov <[email protected]>

Oleg, Roland, you guys are the maintainers, so how do you guys want to route the patches which have been acked?

As it's likely that there
will be quite some number of ptrace patches, it will be better to have a git tree.

Oleg, Roland, you guys are the maintainers, so how do you guys want to route the patches which have been acked? As it's likely that there will be quite some number of ptrace patches, it will be better to have a git tree.

Quoted 4 lines

Oleg, Roland, you guys are the maintainers, so how do you guys want to route the patches which have been acked? As it's likely that there will be quite some number of ptrace patches, it will be better to have a git tree.

In practice we are never trusted enough with changes in this area to get them just pulled from a tree of ours. They have to go through akpm and Linus for individual approval before they get in. So I see no point in me or Oleg maintaining a git tree that won't get merged directly anyway.

The wake_up_process() call in ptrace_detach() is spurious and not interlocked with the tracee state. IOW, the tracee could be running or sleeping in any place in the kernel by the time wake_up_process() is called. This can lead to the tracee waking up unexpectedly which can be dangerous.

The wake_up is spurious and should be removed but for now reduce its toxicity by only waking up if the tracee is in TRACED or STOPPED state.

Signed-off-by Tejun Heo <[email protected]>

Cc Roland McGrath <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc [email protected]

Index: work/kernel/ptrace.c
===================================================================

kernel/ptrace.c +1 -1

@@ -313,7 +313,7 @@ int ptrace_detach(struct task_struct *ch
 		child->exit_code = data;
 		dead = __ptrace_detach(current, child);
 		if (!child->exit_state)
-			wake_up_process(child);
+			wake_up_state(child, TASK_TRACED | TASK_STOPPED);
 	}
 	write_unlock_irq(&tasklist_lock);
 

Am unable to work out why you tagged it for backporting. It fixes some observed bug? Perhaps a regression?

Quoted 2 lines

Am unable to work out why you tagged it for backporting. It fixes some observed bug? Perhaps a regression?

No observed bug, only theoretical ones (AFAIK, never even a ginned-up synthetic test case has been demonstrated). Certainly not a regression, since it has been this (wrong) way since the dawn of time. I don't think this first change is dangerous for -stable, but I have seen no positive rationale for pushing it there.

Quoted 3 lines · On Tue, 1 Feb 2011 21:33:31 -0800 (PST) Roland McGrath <[email protected]>…

Quoted 2 lines

Am unable to work out why you tagged it for backporting. It fixes some observed bug? Perhaps a regression?

Quoted 6 lines

No observed bug, only theoretical ones (AFAIK, never even a ginned-up synthetic test case has been demonstrated). Certainly not a regression, since it has been this (wrong) way since the dawn of time. I don't think this first change is dangerous for -stable, but I have seen no positive rationale for pushing it there.

OK, thanks. I shall destabilize my copy of this patch.

Hello,

Quoted 5 lines · Andrew Morton

Quoted 3 lines · On Tue, 1 Feb 2011 21:33:31 -0800 (PST) Roland McGrath <[email protected]>…

Quoted 2 lines

Am unable to work out why you tagged it for backporting. It fixes some observed bug? Perhaps a regression?

Quoted 8 lines

Quoted 6 lines

No observed bug, only theoretical ones (AFAIK, never even a ginned-up synthetic test case has been demonstrated). Certainly not a regression, since it has been this (wrong) way since the dawn of time. I don't think this first change is dangerous for -stable, but I have seen no positive rationale for pushing it there.

OK, thanks. I shall destabilize my copy of this patch.

It can be used as an attack vector. I don't think it will take too much effort to come up with an attack which triggers oops somewhere. Most sleeps are wrapped in condition test loops and should be safe but we have quite a number of places where sleep and wakeup conditions are expected to be interlocked. Although the window of opportunity is tiny, ptrace can be used by non-privileged users and with some loading the window can definitely be extended and exploited.

The chance of this problem being visible under normal usage is extremely low so no wonder there is no related bug report but that is very different from being safe against targeted attacks.

As the likelihood of causing user noticeable breakage is very low, I think we better push it through -stable.

We're learning some lessons about changelogging here :(

I added this:

Quoted 3 lines

Am unable to work out why you tagged it for backporting. It fixes some observed bug? Perhaps a regression?

No observed bug,

I don't think
this first change is dangerous for -stable, but I have seen no positive rationale for pushing it there.

Yeap, sounds reasonable. So, you're saying it can go away but should be converted to safer sleep first, right? I'll resequence the patches.

ptrace, add @why to ptrace_stop(). Existing users are updateda such

It feels nasty to add a word to task_struct just for this. I don't see another place to store such bookkeeping bits.

But I'm not entirely convinced that we'll really need them when we conclude on fully cleaning up the whole picture.

I have plans on separate out ptrace related stuff from task_struct so that they're allocate iff they're used which will save some tens of bytes on the task struct, so there at least is a plan to compensate for the added cruft.

Quoted 2 lines

But I'm not entirely convinced that we'll really need them when we conclude on fully cleaning up the whole picture.

I really don't know at this point. I tried to make it share one of the related fields but there needs to be a per-task field which is protected by siglock and there currently isn't any, so...

Quoted 4 lines

I have plans on separate out ptrace related stuff from task_struct so that they're allocate iff they're used which will save some tens of bytes on the task struct, so there at least is a plan to compensate for the added cruft.

My, that sounds familiar. Oleg and I have pursued that before, though not in exactly the same context. It sure gets complicated quickly in the corners. But we would still like to see it get done one way or another.

Quoted 6 lines

Quoted 2 lines

But I'm not entirely convinced that we'll really need them when we conclude on fully cleaning up the whole picture.

I really don't know at this point. I tried to make it share one of the related fields but there needs to be a per-task field which is protected by siglock and there currently isn't any, so...

My point was that I am not yet convinced we'll need any new bookkeeping of that sort once we fully work out the group-stop interactions. We're still discussing that in the other threads. So we'll see where that goes.

A visible behavior change is increased likelihood of delayed group stop completion if the thread group contains one or more ptraced tasks.

Quoted 3 lines

A visible behavior change is increased likelihood of delayed group stop completion if the thread group contains one or more ptraced tasks.

I object to that difference in behavior. As I've said before, I don't think there should be any option to circumvent a group stop via ptrace. If you think otherwise, you have a hard road to convince me of it.

It was always the intent that traced tasks should participate in the group stop bookkeeping. I suspect the better line of fixes will be just to tie up the loose ends of the ptrace interactions so that all ptrace stops do the correct group_stop_count bookkeeping and notifications.

If there is a group stop in progress but not yet complete, then PTRACE_CONT on a thread in the group should probably just move it from TASK_TRACED to TASK_STOPPED without resuming it at all.

Once a group stop is complete, then probably the ideal is that PTRACE_CONT would not resume a thread until a real SIGCONT cleared the job control stop condition. But it's likely that existing ptrace users have expectations contrary to that, so we'll have to see.

Yes, I do have some other ideas. When a ptraced task gets a stop signal, its delivery is controlled by the tracer, right? So, right from the beginning, group stop having consistent precedence over ptrace breaks.

As long as the interaction is consistent and well defined, I don't really think it matters one way or the other but given the above precedence and the current ptracers' expectations, I can't see how we would be able to prioritize group stop over ptrace at this point.

The problem is that those loose ends can't be tied up without breaking the current users. PTRACE_CONT has priority over group stop and it's a very visible from userland. I'm afraid the window of opportunity to make that behavior the default had already passed quite some time ago.

Quoted 3 lines

If there is a group stop in progress but not yet complete, then PTRACE_CONT on a thread in the group should probably just move it from TASK_TRACED to TASK_STOPPED without resuming it at all.

I really don't think that's an option at this point and can't see how such behavior could be made consistent given ptracer has inherent superiority over signal delivery. That would make initiation of group stop controllerd by ptracer but participation not. The behavior becomes essentially indeterministic depending on which task in the group gets the signal. :-(

Quoted 5 lines

Once a group stop is complete, then probably the ideal is that PTRACE_CONT would not resume a thread until a real SIGCONT cleared the job control stop condition. But it's likely that existing ptrace users have expectations contrary to that, so we'll have to see.

So, no, I don't think that would be possible or even desirable.

Quoted 4 lines

The problem is that those loose ends can't be tied up without breaking the current users. PTRACE_CONT has priority over group stop and it's a very visible from userland. I'm afraid the window of opportunity to make that behavior the default had already passed quite some time ago.

I am not convinced of that at all, though I certainly wouldn't say now that it's a settled question yet. The userland expectations are pretty convoluted too. I suspect that what you are calling the userland expectations for PTRACE_CONT to ignore the state of a pending group stop are in fact just fallout of userland confusion about what's a job control stop and what's a ptrace stop.

I don't think I follow your logic and I certainly don't agree with your conclusion. It's simple: if a stop signal is actually delivered, then every thread stops. If one thread is traced and another is not, then the tracer can prevent a signal from being delivered to one thread and cannot prevent it from being delivered to another.

Quoted 6 lines

Quoted 5 lines

Once a group stop is complete, then probably the ideal is that PTRACE_CONT would not resume a thread until a real SIGCONT cleared the job control stop condition. But it's likely that existing ptrace users have expectations contrary to that, so we'll have to see.

Quoted 1 line

So, no, I don't think that would be possible or even desirable.

Of course it's possible. We have to work out the entire web of assumptions and ramifications to be sure what's desireable given practical compatibility constraints. I think it's just plain obvious that it's the desireable thing in the abstract--that tracing stops and job control stops should be independent functions.

You'll have to show a lot more details on how to untangle "the entire web of assumptions and ramifications" so that we can reverse the current priority without causing noticeable behavior differences to the existing users. I don't agree what you suggest is "plain obviously desirable" but that's almost beside the point. I really can't see how that would be realistically possible at this point.

So, can you please elaborate how to reverse that and at the same time avoid breaking existing assumptions?

If
there is a group stop in progress but not yet complete, then PTRACE_CONT on a thread in the group should probably just move it from TASK_TRACED to TASK_STOPPED without resuming it at all.

Once a group stop is complete, then probably the ideal is that PTRACE_CONT would not resume a thread until a real SIGCONT cleared the job control stop condition.

But it's likely that existing ptrace users
have expectations contrary to that,

Currently, if the task is STOPPED on ptrace attach, it's left alone and the state is silently changed to TRACED on the next ptrace call.

IMHO this sort of band-aid does not really help the overall situation. It takes something that is intricate and fiddly and just fiddles it a bit more. Userland will still have to handle older kernels where this behavior is not there. If userland does anything that relies on this new behavior, then it will have to try somehow to figure out which kernel versions have which behavior and adapt, etc.

When the old behaviors are unhelpful like this, I think it is really better to add new mechanisms instead.

Quoted 7 lines · Roland McGrath

IMHO this sort of band-aid does not really help the overall situation. It takes something that is intricate and fiddly and just fiddles it a bit more. Userland will still have to handle older kernels where this behavior is not there. If userland does anything that relies on this new behavior, then it will have to try somehow to figure out which kernel versions have which behavior and adapt, etc.

Absolutely agreed.

As I said, I am not sure this patch makes sense. I only sent it because I have to react to the bug report.

Quoted 2 lines

When the old behaviors are unhelpful like this, I think it is really better to add new mechanisms instead.

Agreed!

Can't resist, let me repeat... imho ptrace is unfixable ;)

Hello, Oleg, Roland.

Quoted 14 lines · Oleg Nesterov

Quoted 7 lines · Roland McGrath

IMHO this sort of band-aid does not really help the overall situation. It takes something that is intricate and fiddly and just fiddles it a bit more. Userland will still have to handle older kernels where this behavior is not there. If userland does anything that relies on this new behavior, then it will have to try somehow to figure out which kernel versions have which behavior and adapt, etc.

Absolutely agreed.

As I said, I am not sure this patch makes sense. I only sent it because I have to react to the bug report.

Quoted 6 lines

Quoted 2 lines

When the old behaviors are unhelpful like this, I think it is really better to add new mechanisms instead.

Agreed!

Can't resist, let me repeat... imho ptrace is unfixable ;)

Hmm... I can't reproduce the problem here,

but isn't the problematic
part here the mixing of ptrace and group stop and sliently transforming group stop into ptrace

and ptracer consuming the usual
exit code instead of the ptrace specific one?

Also, I don't agree with the notion that doing something entirely new would magically solve all the problems. Improvements are achieved through evolution. For ptrace, the situation definitely is aggravated by the use of wait

and weird interaction with group stop,

Actually, I think such approach is significantly harmful to improvements of the existing code base. Instead of encouraging investigating the actual problems and making sensible tradeoffs, such approach discourages making reasonable tradeoffs with the false expectation that something in the future will magically solve the problems. In a lot of cases, there are no unicorns. Proceeding forward while managing damage at reasonable level is usually the right way to go.

Quoted 1 line

Hmm... I can't reproduce the problem here,

Very strange. Do you mean the test-case doesn't die? (on vanilla kernel).

Quoted 3 lines

but isn't the problematic
part here the mixing of ptrace and group stop and sliently transforming group stop into ptrace

Not exactly,

Quoted 2 lines

and ptracer consuming the usual
exit code instead of the ptrace specific one?

Well, unless the task dies nobody except ptrace can use ->exit_code.

The problem is:

- the task T stops, it sets ->exit_code exactly because the tracer can attach after that

- the tracer attaches, does wait(), consumes exit_code and exits

- another tracer attaches, but exit_code == 0

There is no STOPPED/TRACED transformation at all.

Quoted 4 lines

Also, I don't agree with the notion that doing something entirely new would magically solve all the problems. Improvements are achieved through evolution. For ptrace, the situation definitely is aggravated by the use of wait

... and reparenting, and signals.

Quoted 1 line

and weird interaction with group stop,

Yes. And to me the main problem is not the current behaviour. The problem is that we never tried to define the correct behavior. OK, real_parent can miss the notification. We can fix this, but for what? The tracer can resume the thread "silently", this doesn't look very good anyway.

But even this doesn't matter. We can not change ptrace API so that, say, it does not reparent the tracee. Once we do this, we already have the new API.

So, personally I think we need the new API. And we already have utrace which allows to implement "anything" on top of it, including the old ptrace for compatibility.

Well, perhaps I am wrong, this is only my opinion.

Hey, guys.

Quoted 3 lines · Oleg Nesterov

Quoted 1 line

Hmm... I can't reproduce the problem here,

Very strange. Do you mean the test-case doesn't die? (on vanilla kernel).

Heh, it turns out the second child was attaching before the first succeeded stopping itself, so when it gets detached for the first time, the first child then stops generating new exit_code.

Adding a
small delay to the parent after the first child started made it reliably fail on the vanially kernel.

Quoted 1 line

There is no STOPPED/TRACED transformation at all.

But it is. It happens because there is no clear distinction between group stop and ptrace_stop.

With my first series applied, it doesn't
happen anymore because ptracer _never_ depends on or consumes group stop exit_code.

Yes, I agree it's ugly but that's what we already have. I think we can still achieve well-defined behavior even with ptracer allowed to diddle with the task while group stop is in effect. It may not be immediately intuitive but I personally think it actually would be more useful to do things that way, as long as we clearly lay out what are supported what are undefined.

I think a good compromise would be guaranteeing that when the ptracer goes away, the tracee would put into the state the real parent can agree to and the real parent to be notified that it has happened.

We
are already skipping all notifications to the real parent for ptraced children, there's no pressing need to change that. If there becomes a real pressing requirement to change that.

I could be wrong (with pretty high probability) but I don't really see the pressing need for a completely new API. ptrace sure is ugly and quirky but it's something people are already used to.

Quoted 3 lines

I think a good compromise would be guaranteeing that when the ptracer goes away, the tracee would put into the state the real parent can agree to and the real parent to be notified that it has happened.

I am not sure. The tracing should be transparent as much as possible.

Quoted 3 lines

We are already skipping all notifications to the real parent for ptraced children, there's no pressing need to change that. If there becomes a real pressing requirement to change that.

And this looks just wrong to me. Say, why the ptraced application doesn't react to ^Z ? It does, just it parent have no chance to see this. (yes, yes, we should also change do_wait).

Quoted 3 lines

I could be wrong (with pretty high probability) but I don't really see the pressing need for a completely new API. ptrace sure is ugly and quirky but it's something people are already used to.

I won't argue. And yes, in any case ptrace can't go away, we should try to improve it anyway. The obvious problem is that almost any "visible" improvement breaks something.

Quoted 7 lines · Oleg Nesterov

Quoted 3 lines

We are already skipping all notifications to the real parent for ptraced children, there's no pressing need to change that. If there becomes a real pressing requirement to change that.

And this looks just wrong to me. Say, why the ptraced application doesn't react to ^Z ? It does, just it parent have no chance to see this. (yes, yes, we should also change do_wait).

That's the shortcomings of the current implementation. The specific problem sure can be fixed by putting group stop on top of ptrace but that is not the only direction. In fact, that actually is the direction we CAN'T take with ptrace because changing that will create a lot more problems that can't be worked around.

We can introduce something completely new but we can also augment the current implementation with what's necessary to remedy the problem. The ptracer is already notified when the ptracee enters group stop. There's nothing stopping us giving ptracer the ability to tell ptracee to participate in group stop completion and notify the real parent. It will be an extra feature, probably a new PTRACE_ operation.

This
way, the existing users behave the same way

while the ones which are
updated to use the new feature would behave better w.r.t. group stop while being ptraced.

"oh, we'll have something completely new which cures everything at once, so let's reject changes to the current code as much as possible"

For example, the problem in this thread is cleanly solved by really examining the problem and fixing the problem at the source (the mixup of group and ptrace stop)

So, let's stop chasing unicorns. Cake is a lie. Let's work on what we already have and incrementally improve it.

Quoted 5 lines

That's the shortcomings of the current implementation. The specific problem sure can be fixed by putting group stop on top of ptrace but that is not the only direction. In fact, that actually is the direction we CAN'T take with ptrace because changing that will create a lot more problems that can't be worked around.

Which problems?

Quoted 1 line

This way, the existing users behave the same way

Wait. The current behaviour is just broken. This is bad. But, at the same time, this is good: it gives us more rights to introduce the user-visible changes once we decide what exactly we want.

Firstly, the current behaviour is unpredictable. Suppose that we have two threads, T1 and T2. Only one thread is ptraced. Now, real_parent will be notified or not, depending on which thread calls do_signal_stop() last. I see no point to preserve this randomness. And note that real_parent can be notified anyway.

Otoh, I don't really understand why should we delay the notification to real_parent until detach in the simplest case (with your patches) by default.

Quoted 3 lines

For example, the problem in this thread is cleanly solved by really examining the problem and fixing the problem at the source (the mixup of group and ptrace stop)

Yes, but I am worried that this change (in its current form) makes impossible to create a TASK_STOPPED tracee, but you already know this.

OK. But what I can't understand is why the alternative change is not better. Once again:

- the stopping thread always notifies the debugger

- the last thread notifies both: debugger and real_parent

- do_wait() is modified so that WSTOPPED always works for real_parent, even if its child is ptraced.

Hello, Oleg.

Quoted 6 lines · Oleg Nesterov

Quoted 5 lines

That's the shortcomings of the current implementation. The specific problem sure can be fixed by putting group stop on top of ptrace but that is not the only direction. In fact, that actually is the direction we CAN'T take with ptrace because changing that will create a lot more problems that can't be worked around.

Quoted 1 line

Which problems?

I was talking about prioritizing group stop over ptrace in general. Please see the following messages.

http://article.gmane.org/gmane.linux.kernel/1095119
http://article.gmane.org/gmane.linux.kernel/1095603

Notifying the parent w/o making group stop superior to ptrace sure is a possibility.

Quoted 6 lines

Quoted 3 lines

For example, the problem in this thread is cleanly solved by really examining the problem and fixing the problem at the source (the mixup of group and ptrace stop)

Yes, but I am worried that this change (in its current form) makes impossible to create a TASK_STOPPED tracee, but you already know this.

Why is that a problem?

A ptraced task stops in TASK_TRACED.

Quoted 5 lines

OK. But what I can't understand is why the alternative change is not better. Once again:

- the stopping thread always notifies the debugger

Quoted 4 lines

- the last thread notifies both: debugger and real_parent

- do_wait() is modified so that WSTOPPED always works for real_parent, even if its child is ptraced.

I think the disconnection comes from the scope of the problem. If we restrict our attention to group stop notification.

I agree that what
you're describing seems like a good compromise. What I was objecting to was putting group stop mechanism in general on top of ptrace. I can't see how that would work.

Also, for a ptraced task, what would you consider to be participating in a group stop?

I think it should only include the case where the tracee actually stops for group stop excluding all other trapping points.

But, I don't think this really changes the need for state tracking. We would still have to put the tracee into approriate mode on detach.

Quoted 2 lines · Tejun Heo

http://article.gmane.org/gmane.linux.kernel/1095119
http://article.gmane.org/gmane.linux.kernel/1095603

Yes, I tried to read this... But I have to admit I can hardly understand your discussion with Roland. More precisely, I don't understand what exactly you have in mind.

One (may be off-topic) note,

	On 01/31, Tejun Heo wrote:
	>
	> On Fri, Jan 28, 2011 at 01:30:09PM -0800, Roland McGrath wrote:
	> > > A visible behavior change is increased likelihood of delayed group
	> > > stop completion if the thread group contains one or more ptraced
	> > > tasks.
	> >
	> > I object to that difference in behavior.  As I've said before, I don't
	> > think there should be any option to circumvent a group stop via ptrace.
	> > If you think otherwise, you have a hard road to convince me of it.

I agree with Roland here.

> Yes, I do have some other ideas. When a ptraced task gets a stop > signal, its delivery is controlled by the tracer, right?

Right, but note that the tracer does not fully control the group-stop. One a thread dequeues SIGSTOP (and please note this thread can be !traced), all other threads (traced or not) should participate.

As for SIGCONT priority, see below.

Quoted 2 lines

Notifying the parent w/o making group stop superior to ptrace sure is a possibility.

Could you please reiterate? I think I missed something before, and now I do not really understand what do you mean.

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

For example, the problem in this thread is cleanly solved by really examining the problem and fixing the problem at the source (the mixup of group and ptrace stop)

Yes, but I am worried that this change (in its current form) makes impossible to create a TASK_STOPPED tracee, but you already know this.

Quoted 1 line

Why is that a problem?

See above. Because I think ptrace should not "hide" jctl stops (at least by default), and SIGCONT should work in this case.

Quoted 1 line

A ptraced task stops in TASK_TRACED.

Unless it reacts to SIGSTOP/group_stop_count.

Quoted 4 lines

I agree that what
you're describing seems like a good compromise. What I was objecting to was putting group stop mechanism in general on top of ptrace. I can't see how that would work.

And I still can't understand why this can't work ;)

And I don't really understand "putting group stop mechanism in general on top of ptrace". It is very possible I am wrong, but I see this from the different angle: stop/ptrace should be "parallel".

Quoted 3 lines

I think it should only include the case where the tracee actually stops for group stop excluding all other trapping points.

I was thinking about this too and probably this makes sense. But I think at least initial changes should keep the current behaviour (assuming this behaviour is fixed).

Quoted 2 lines

But, I don't think this really changes the need for state tracking. We would still have to put the tracee into approriate mode on detach.

Sure, but we already have SIGNAL_STOP_STOPPED/group_signal_stop. I meant, we do not need to remember the state per-thread.

As for SIGCONT. Roland suggests (roughly) to change ptrace_resume() so that it doesn't wakeup the stopped tracee until the real SIGCONT comes. And this makes sense to me.

> For example, if a gdb traced task is instructed to participate in a > group stop and then hits a ptrace trap, it would have to participate > in the group stop as it enters ptrace trap, right? gdb's wait(2) > would complete indicating ptrace trap. After the user tells the task > to continue, the task shouldn't resume until SIGCONT is received;

Yes. But to me, this looks correct! The tracee shouldn't resume exactly because it is stopped.

> however, at this point, there's no way for gdb to tell what's going on > with the tracee.

Yes. I think this should be improved somehow, currently gdb can only look in /proc/tid/status to detect this case.

> If ptrace behaved like that from the beginning, gdb would have behaved > differently and worked around those cases but that hasn't been the > case

Cough... I thought we agreed it is better to break some corner cases but make ptrace more consistent ;)

But yes, I see your point. And while I think that Roland's suggestion is fine, I also have another proposal

- never send CLD_CONTINUED to the tracer, always send it to parent.

Firstly, this is completely pointless: ptrace is per-thread, while this notification is per-process

- change do_wait() so that WCONTINUED for real_parent

- change ptrace_resume() to check SIGNAL_STOP_STOPPED case. It should act as SIGCONT in this case. Yes: "act as SIGCONT" needs more discussion.

Hey, Oleg.

Quoted 4 lines · Oleg Nesterov

> Yes, I do have some other ideas. When a ptraced task gets a stop > signal, its delivery is controlled by the tracer, right?

Quoted 3 lines

Right, but note that the tracer does not fully control the group-stop. One a thread dequeues SIGSTOP (and please note this thread can be !traced), all other threads (traced or not) should participate.

I don't know. Maybe it's more consistent that way and I'm not fundamentally against that but it is a big behavior change

Quoted 5 lines

Quoted 2 lines

Notifying the parent w/o making group stop superior to ptrace sure is a possibility.

Could you please reiterate? I think I missed something before, and now I do not really understand what do you mean.

I was trying to say that it's still possible to deliver group stop notifications to the real parent while letting the ptracer override group stop with PTRACE_CONT.

Quoted 3 lines

Quoted 1 line

A ptraced task stops in TASK_TRACED.

Unless it reacts to SIGSTOP/group_stop_count.

What do you do about PTRACE requests while a task is group stopped? Reject them? Block them?

Quoted 5 lines

Quoted 4 lines

I agree that what
you're describing seems like a good compromise. What I was objecting to was putting group stop mechanism in general on top of ptrace. I can't see how that would work.

Quoted 5 lines

And I still can't understand why this can't work ;)

And I don't really understand "putting group stop mechanism in general on top of ptrace". It is very possible I am wrong, but I see this from the different angle: stop/ptrace should be "parallel".

Hmmm... currently ptrace overrides group stop and has full control over when and where the tracee stops and continues,

I don't think it's an extreme corner case
we can break. For example, if a user gdb's a program which raises one of the stop signals, currently the user expects to be able to continue the program from whithin the gdb. If we make group stop override ptrace, there's no other recourse than sending signal from outside.

Quoted 4 lines

Quoted 3 lines

I think it should only include the case where the tracee actually stops for group stop excluding all other trapping points.

Quoted 3 lines

I was thinking about this too and probably this makes sense. But I think at least initial changes should keep the current behaviour (assuming this behaviour is fixed).

But if you make the other change but not this one, we end up with ptrace which doesn't notify the ptracer what's going on. Apart from _polling_ /proc/tid/status, there is no mechanism to discover the tracee's state.

The only thing it achieves is the integrity of group stop

and I'm not really sure whether that's something worth achieving at the cost of debugging capabilities especially when we don't _have_ to lose them.

Quoted 3 lines

As for SIGCONT. Roland suggests (roughly) to change ptrace_resume() so that it doesn't wakeup the stopped tracee until the real SIGCONT comes. And this makes sense to me.

I agree it adds more integrity to group stop but at the cost of debugging capabilities. I'm not yet convinced integrity of group stop is that important. Why is it such a big deal?

Quoted 7 lines

But yes, I see your point. And while I think that Roland's suggestion is fine, I also have another proposal

- never send CLD_CONTINUED to the tracer, always send it to parent.

Firstly, this is completely pointless: ptrace is per-thread, while this notification is per-process

CLD_STOPPED is too but while ptrace is attached the notifications are made per-task and delivered to the tracer.

Quoted 5 lines

- change do_wait() so that WCONTINUED for real_parent

- change ptrace_resume() to check SIGNAL_STOP_STOPPED case. It should act as SIGCONT in this case. Yes: "act as SIGCONT" needs more discussion.

I don't know.

I think this is the key question. Whether to de-throne PTRACE_CONT such that it cannot override group stop. As I've already said several times already, I think it is a pretty fundamental property of ptrace

and change to it would be quite visible,

in
negative way, from userland.

Quoted 2 lines

I don't know. Maybe it's more consistent that way and I'm not fundamentally against that but it is a big behavior change

Hmm. I tried to describe the current behaviour...

Quoted 2 lines

What do you do about PTRACE requests while a task is group stopped? Reject them? Block them?

Yes, another known oddity. Of course we shouldn't reject or block. Perhaps we can ignore this initially. If SIGCONT comes after another request does STOPPED/TRACED it clears SIGNAL_STOP_STOPPED, but the tracee won't run until the next PTRACE_CONT, this makes sense.

The problem is, gdb can't leave the tracee in STOPPED state if it wants. We need to improve this somehow (like in your previous example with gdb).

Only if it attaches to every thread in the thread group. Otherwise, if the non-thread has already initiated the group-stop, the tracee will notice TIF_SIGPENDING eventually and call do_signal_stop(), debugger can't control this.

Quoted 5 lines

I don't think it's an extreme corner case
we can break. For example, if a user gdb's a program which raises one of the stop signals, currently the user expects to be able to continue the program from whithin the gdb. If we make group stop override ptrace, there's no other recourse than sending signal from outside.

Yes. Of course, gdb can be "fixed", it can send SIGCONT.

But yes, this is the noticeable change, that is why I suggested ptrace_resume-acts-as-SIGCONT logic. Ugly, yes, but more or less compatible. (although let me repeat, _pesonally_ I'd prefer to simply tell user-space to learn the new rules ;)

Quoted 2 lines

The only thing it achieves is the integrity of group stop

Given that SIGCHLD doesn't queue and with or without your changes we send it per-thread, it is not trivial for gdb to detect the group-stop anyway. Again, the kernel should help somehow.

Quoted 3 lines

and I'm not really sure whether that's something worth achieving at the cost of debugging capabilities especially when we don't _have_ to lose them.

But we do not? I mean, at least this is not worse than the current behaviour.

Quoted 7 lines

Quoted 3 lines

As for SIGCONT. Roland suggests (roughly) to change ptrace_resume() so that it doesn't wakeup the stopped tracee until the real SIGCONT comes. And this makes sense to me.

I agree it adds more integrity to group stop but at the cost of debugging capabilities. I'm not yet convinced integrity of group stop is that important. Why is it such a big deal?

Of course I can't "prove" it is that important. But I think so.

Quoted 2 lines

CLD_STOPPED is too but while ptrace is attached the notifications are made per-task and delivered to the tracer.

No, there is a difference. Sure, CLD_STOPPED is per-process without ptrace. But CLD_CONTINUED continues to be per-process even if all threads are traced.

Quoted 4 lines

I think this is the key question. Whether to de-throne PTRACE_CONT such that it cannot override group stop. As I've already said several times already, I think it is a pretty fundamental property of ptrace

Again, I am a bit confused. Note that PTRACE_CONT overrides group stop if we do the above. It should wake up the tracee, in SIGCONT-compatible way (yes, the latter is not exactly clear).

But at least this should be visible to real parent. We shouldn't silently make the stopped tracee running while its real_parent thinks everything is stopped.

Heh, I'm asking for proof that it is more useful. :-) But I'm still

We can make it behave like the following. { | } denotes two alternative behaviors regarding SIGCONT.

If a group stop is initiated while, or was in progress when a task is ptraced, the task will stop for group stop and notify the ptracer accordingly. Note that the task could be trapped elsewhere delaying this from happening. When the task stops for group stop, it participates in group stop as if it is not ptraced and the real parent is notified of group stop completion.

Note that { the task is put into TASK_TRACED state and group stop resume by SIGCONT is ignored. | the task is put into TASK_STOPPED state and the following PTRACE request will transition it into TASK_TRACED. If SIGCONT is received before transition to TASK_TRACED is made, the task will resume execution. If PTRACE request faces with SIGCONT, PTRACE request may fail. }

The ptracer may resume execution of the task using PTRACE_CONT without affecting other tasks in the group.

On ptrace detach, if group stop is in effect, the task will be put into TASK_STOPPED state and if it is the first time the task is stopping for the current group stop, it will participate in group stop completion.

This can be phrased better but it seems well defined enough for me. I take it that one of your concerns is direct transition into TASK_TRACED on group stop while ptraced which prevents the tracee from reacting to the following SIGCONT.

I'm not sure how much of an actual
problem it is given that our notification to real parent hasn't worked at all till now

but we can definitely implement proper TASK_STOPPED -> TRACED transition on the next PTRACE request.

There exists a
fundamental race condition between SIGCONT and the next PTRACE call

If we don't go that route, another solution would be to add a ptrace call which can listen to SIGCONT. ie. PTRACE_WAIT_CONT or whatever which the ptracer can call once it knows the tracee entered group stop.

In either case, the fundamentals of ptrace operation don't really change. All ptrace operations are still per-task and ptracer almost always has control over execution of the tracee. Sure, it allows ptraced task to escape group stop but it seems defined clear enough and IMHO actually is a helpful debugging feature.

After all, it's not
like stop signals can be used for absoultely reliable job control. There's an inherent race against SIGCONT.

Quoted 7 lines

Quoted 2 lines

What do you do about PTRACE requests while a task is group stopped? Reject them? Block them?

Yes, another known oddity. Of course we shouldn't reject or block. Perhaps we can ignore this initially. If SIGCONT comes after another request does STOPPED/TRACED it clears SIGNAL_STOP_STOPPED, but the tracee won't run until the next PTRACE_CONT, this makes sense.

That conceptually might make sense

but other than the conceptual
integrity it widely changes the assumptions and is less useful than the current behavior.

I don't really see why we would want to do
that.

Quoted 4 lines

Only if it attaches to every thread in the thread group. Otherwise, if the non-thread has already initiated the group-stop, the tracee will notice TIF_SIGPENDING eventually and call do_signal_stop(), debugger can't control this.

The debugger is still notified and can override it.

Quoted 8 lines

Quoted 5 lines

I don't think it's an extreme corner case
we can break. For example, if a user gdb's a program which raises one of the stop signals, currently the user expects to be able to continue the program from whithin the gdb. If we make group stop override ptrace, there's no other recourse than sending signal from outside.

Yes. Of course, gdb can be "fixed", it can send SIGCONT.

Quoted 4 lines

But yes, this is the noticeable change, that is why I suggested ptrace_resume-acts-as-SIGCONT logic. Ugly, yes, but more or less compatible. (although let me repeat, _pesonally_ I'd prefer to simply tell user-space to learn the new rules ;)

I can't really agree there. First, to me, it seems like too radical a change

and secondly the resulting behavior might look conceptually pleasing but is not as useful as the current one. Why make a change which results in reduced usefulness while noticeably breaking existing users?

Quoted 3 lines

Given that SIGCHLD doesn't queue and with or without your changes we send it per-thread, it is not trivial for gdb to detect the group-stop anyway. Again, the kernel should help somehow.

Hmmm? Isn't this discoverable from the exit code from wait?

Quoted 6 lines

Quoted 3 lines

and I'm not really sure whether that's something worth achieving at the cost of debugging capabilities especially when we don't _have_ to lose them.

But we do not? I mean, at least this is not worse than the current behaviour.

I think it's worse. With your changes, debuggers can't diddle the tasks behind group stop's back which the current users already expect.

Quoted 5 lines

Quoted 3 lines

I agree it adds more integrity to group stop but at the cost of debugging capabilities. I'm not yet convinced integrity of group stop is that important. Why is it such a big deal?

Of course I can't "prove" it is that important. But I think so.

Heh, I'm not asking for proof that it is more useful. :-) But I'm still curious why you think it's important because the benefits aren't apparent to me. Roland and you seem to share this opinion without much dicussion so maybe I'm missing something?

Quoted 6 lines

Quoted 2 lines

CLD_STOPPED is too but while ptrace is attached the notifications are made per-task and delivered to the tracer.

No, there is a difference. Sure, CLD_STOPPED is per-process without ptrace. But CLD_CONTINUED continues to be per-process even if all threads are traced.

Hmm... I need to think more about it. I'm not fully following your point.

Quoted 8 lines

Quoted 4 lines

I think this is the key question. Whether to de-throne PTRACE_CONT such that it cannot override group stop. As I've already said several times already, I think it is a pretty fundamental property of ptrace

Again, I am a bit confused. Note that PTRACE_CONT overrides group stop if we do the above. It should wake up the tracee, in SIGCONT-compatible way (yes, the latter is not exactly clear).

What do you mean? Waking up in SIGCONT-compatible way? Sending SIGCONT ending the whole group stop?

Quoted 3 lines

But at least this should be visible to real parent. We shouldn't silently make the stopped tracee running while its real_parent thinks everything is stopped.

I think maybe this is where our different POVs come from.

To me, it
isn't too objectionable to allow debuggers to diddle with tracees behind the real parent's back. In fact, it would be quite useful when debugging job control related behaviors. I wouldn't have much problem accepting the other way around - ie. strict job control even while being debugged, but given that it is already allowed and visible, I fail to see why we should change the behavior. It doesn't seem to have enough benefits to warrant such visible change.

If I change the patchset such that group stop while ptraced first enters TASK_STOPPED and then transitions into TASK_TRACED on the next PTRACE call,

there will be race window which would be visible

Quoted 6 lines

Note that { the task is put into TASK_TRACED state and group stop resume by SIGCONT is ignored. | the task is put into TASK_STOPPED state and the following PTRACE request will transition it into TASK_TRACED. If SIGCONT is received before transition to TASK_TRACED is made, the task will resume execution. If PTRACE request faces with SIGCONT, PTRACE request may fail. }

To me, the first variant looks better. But, only because it is closer to the current behaviour. I mean, it is better to change the things incrementally.

But in the longer term - I do not know. Personally, I like the TASK_STOPPED variant. To the point, I was thinking that (perhaps) we can change ptrace_stop() so that it simply calls do_signal_stop() if it notices ->group_stop_count != 0.

Quoted 2 lines

The ptracer may resume execution of the task using PTRACE_CONT without affecting other tasks in the group.

And this is what I do not like. I just can't accept the fact there is a running thread in the SIGNAL_STOP_STOPPED group.

But yes: this is what the current code does, I am not sure we can change this, and both PTRACE_CONT-doesnt-resume-until-SIGCONT and PTRACE_CONT-acts-as-SIGCONT are not "perfect" too.

Quoted 2 lines

There exists a
fundamental race condition between SIGCONT and the next PTRACE call

Yes, and this race is already here, ptracer should take care.

From the API POV, there is no race, if we assume Oleg's interpretation

Quoted 5 lines

In either case, the fundamentals of ptrace operation don't really change. All ptrace operations are still per-task and ptracer almost always has control over execution of the tracee. Sure, it allows ptraced task to escape group stop but it seems defined clear enough and IMHO actually is a helpful debugging feature.

Heh, I think we found the place where we can't convince each other. What if we toss a coin?

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

What do you do about PTRACE requests while a task is group stopped? Reject them? Block them?

Yes, another known oddity. Of course we shouldn't reject or block.

Quoted 4 lines

Heh, I'm not asking for proof that it is more useful. :-) But I'm still curious why you think it's important because the benefits aren't apparent to me. Roland and you seem to share this opinion without much dicussion so maybe I'm missing something?

I can't!

I hate this from the time when I noticed that the application doesn't respond to ^Z under strace. And I used strace exactly because I wanted do debug some (I can't recall exactly) problems with jctl. That is all.

Quoted 8 lines

To me, it
isn't too objectionable to allow debuggers to diddle with tracees behind the real parent's back. In fact, it would be quite useful when debugging job control related behaviors. I wouldn't have much problem accepting the other way around - ie. strict job control even while being debugged, but given that it is already allowed and visible, I fail to see why we should change the behavior. It doesn't seem to have enough benefits to warrant such visible change.

All I can say is: sure, I see your point, and perhaps you are right and I am wrong.

I'd really like to force CC list to participate ;)

* sleep runs in nanosleep
* SIGSTOP arrives, strace sees it
* strace logs it and allows it via ptrace(PTRACE_SYSCALL, ..., SIGSTOP)
* sleep process enters group-stop

* nothing happens until some other signal arrives
* say, SIGCONT arrives

* strace logs it and allows it via ptrace(PTRACE_SYSCALL, ..., SIGCONT)

Quoted 4 lines · Denys Vlasenko

* sleep runs in nanosleep
* SIGSTOP arrives, strace sees it
* strace logs it and allows it via ptrace(PTRACE_SYSCALL, ..., SIGSTOP)
* sleep process enters group-stop

The last point breaks the documented behavior of ptrace:
If data is nonzero and not SIGSTOP, it is interpreted as a signal to be delivered to the child; otherwise, no signal is delivered.

I do not see it would affect gdb. strace will change its behavior when SIGSTOP is sent to its tracee although the new behavior may be OK.

It is more a subject of apps compatibility testing with such a kernel change.

Quoted 2 lines

* nothing happens until some other signal arrives
* say, SIGCONT arrives

What if other signal arrives? The tracer probably should not be notified as the tracee is in a group-stop.

Quoted 6 lines · Jan Kratochvil

Quoted 4 lines · Denys Vlasenko

* sleep runs in nanosleep
* SIGSTOP arrives, strace sees it
* strace logs it and allows it via ptrace(PTRACE_SYSCALL, ..., SIGSTOP)
* sleep process enters group-stop

Quoted 3 lines

The last point breaks the documented behavior of ptrace:
If data is nonzero and not SIGSTOP, it is interpreted as a signal to be delivered to the child; otherwise, no signal is delivered.

But SIGSTOP _is_ delivered - that's why sleep process stops.

Quoted 2 lines

What if other signal arrives? The tracer probably should not be notified as the tracee is in a group-stop.

The behavior here ideally should be the same as for non-traced process:
the signals are remembered while process is stopped, and it sees them only after SIGCONT, as demonstrated by the following program

I believe it would be best if debugger sees signals immediately, but when it does ptrace(PTRACE_CONT/SYSCALL, ..., <sig>) in order to send signals to group-stopped tracee, they are queued to it without terminating group-stop. When SIGCONT arrives, ptrace(PTRACE_CONT/SYSCALL, ..., SIGCONT) terminates group-stop and causes all queued signals to be handled (in random order, not necessarily in the order of arrival. Even CONT handler is not guaranteed to be called first, as you see above).

Quoted 4 lines · Denys Vlasenko

* sleep runs in nanosleep
* SIGSTOP arrives, strace sees it
* strace logs it and allows it via ptrace(PTRACE_SYSCALL, ..., SIGSTOP)
* sleep process enters group-stop

The last point breaks the documented behavior of ptrace:

If data is nonzero and not SIGSTOP, it is interpreted as a signal to be delivered to the child; otherwise, no signal is delivered.

Quoted 2 lines

* nothing happens until some other signal arrives
* say, SIGCONT arrives

What if other signal arrives?

The tracer probably should not be notified as
the tracee is in a group-stop.

Quoted 4 lines · Denys Vlasenko

* sleep runs in nanosleep
* SIGSTOP arrives, strace sees it
* strace logs it and allows it via ptrace(PTRACE_SYSCALL, ..., SIGSTOP)
* sleep process enters group-stop

The last point breaks the documented behavior of ptrace:
If data is nonzero and not SIGSTOP, it is interpreted as a signal to be delivered to the child; otherwise, no signal is delivered.

I do not see it would affect gdb. strace will change its behavior when SIGSTOP is sent to its tracee although the new behavior may be OK.

It is more a subject of apps compatibility testing with such a kernel change.

Quoted 4 lines · Oleg Nesterov

But yes: this is what the current code does, I am not sure we can change this, and both PTRACE_CONT-doesnt-resume-until-SIGCONT and PTRACE_CONT-acts-as-SIGCONT are not "perfect" too.

Can you enumerate reasons why each of them are not perfect? I want to understand your thinking better here.

Quoted 1 line

Yes, and this race is already here, ptracer should take care.

From the API POV, there is no race,

Quoted 6 lines

Quoted 5 lines

In either case, the fundamentals of ptrace operation don't really change. All ptrace operations are still per-task and ptracer almost always has control over execution of the tracee. Sure, it allows ptraced task to escape group stop but it seems defined clear enough and IMHO actually is a helpful debugging feature.

Quoted 2 lines

Heh, I think we found the place where we can't convince each other. What if we toss a coin?

I'm with Oleg on this. If debugger wants to terminate group-stop, it should just send SIGCONT, not depend on the obscure feature (it is not documented, right?) that PTRACE_CONT somehow affects group-stop state.

Quoted 3 lines

I hate this from the time when I noticed that the application doesn't respond to ^Z under strace. And I used strace exactly because I wanted do debug some (I can't recall exactly) problems with jctl. That is all.

Recently I had exactly this experience too. It's frustrating.

You just succeeded :)

Quoted 6 lines

Note that { the task is put into TASK_TRACED state and group stop resume by SIGCONT is ignored. | the task is put into TASK_STOPPED state and the following PTRACE request will transition it into TASK_TRACED. If SIGCONT is received before transition to TASK_TRACED is made, the task will resume execution. If PTRACE request faces with SIGCONT, PTRACE request may fail. }

To me, the first variant looks better. But, only because it is closer to the current behaviour. I mean, it is better to change the things incrementally.

But in the longer term - I do not know. Personally, I like the TASK_STOPPED variant. To the point, I was thinking that (perhaps) we can change ptrace_stop() so that it simply calls do_signal_stop() if it notices ->group_stop_count != 0.

Quoted 2 lines

The ptracer may resume execution of the task using PTRACE_CONT without affecting other tasks in the group.

And this is what I do not like. I just can't accept the fact there is a running thread in the SIGNAL_STOP_STOPPED group.

But yes: this is what the current code does, I am not sure we can change this, and both PTRACE_CONT-doesnt-resume-until-SIGCONT and PTRACE_CONT-acts-as-SIGCONT are not "perfect" too.

Quoted 2 lines

I'm not sure how much of an actual problem it is given that our notification to real parent hasn't worked at all till now

Yes! and this is very good argument in favour of all your objections ;)

Quoted 2 lines

but we can definitely implement proper TASK_STOPPED -> TRACED transition on the next PTRACE request.

I guess, you mean that the current code bypasses the ptrace_stop()->arch_ptrace_stop_needed() code while doing s/STOPPED/TRACED ?

Oh, currently I am ignoring this, my only concern is how this all looks to the userland. But this is the good point, and I have to admit that I never realized this is just wrong. Yes, I agree, we should do something, but this is not visible to user-space (except this should fix the bug ;)

Quoted 4 lines

If we don't go that route, another solution would be to add a ptrace call which can listen to SIGCONT. ie. PTRACE_WAIT_CONT or whatever which the ptracer can call once it knows the tracee entered group stop.

Perhaps... Or something else, but surely there is a room for improvements. Fortunately, the changes like this are "safe". I mean, they can break nothing. Just we should try to not make them wrong ;)

Quoted 5 lines

In either case, the fundamentals of ptrace operation don't really change. All ptrace operations are still per-task and ptracer almost always has control over execution of the tracee. Sure, it allows ptraced task to escape group stop but it seems defined clear enough and IMHO actually is a helpful debugging feature.

Heh, I think we found the place where we can't convince each other. What if we toss a coin?

Quoted 2 lines

After all, it's not like stop signals can be used for absoultely reliable job control. There's an inherent race against SIGCONT.

Sure, if we are talking about SIGCONT from "nowhere". But, the same way ^Z is not reliable too.

Quoted 2 lines

but other than the conceptual integrity it widely changes the assumptions and is less useful than the current behavior.

Hmm, this is what we currently have?

Quoted 1 line

The debugger is still notified and can override it.

Hmm... no, it can't? Of course it is notified after the tracee participates and calls do_signal_stop() and gdb can resume it then. But it can't prevent the tracee from stopping.

Quoted 2 lines

I can't really agree there. First, to me, it seems like too radical a change

(I assume you mean PTRACE_CONT-doesnt-resume variant?)

Quoted 5 lines

Quoted 3 lines

Given that SIGCHLD doesn't queue and with or without your changes we send it per-thread, it is not trivial for gdb to detect the group-stop anyway. Again, the kernel should help somehow.

Hmmm? Isn't this discoverable from the exit code from wait?

Sure. Probably I misunderstood. I thought, you mean we need something like per-process "the whole group is stopped" notification for the debugger.

Quoted 2 lines

I think it's worse. With your changes, debuggers can't diddle the tasks behind group stop's back which the current users already expect.

OK, I certainly misunderstood you, and now I can't restore the context. Could you spell?

Quoted 4 lines

Heh, I'm not asking for proof that it is more useful. :-) But I'm still curious why you think it's important because the benefits aren't apparent to me. Roland and you seem to share this opinion without much dicussion so maybe I'm missing something?

I can't!

I hate this from the time when I noticed that the application doesn't respond to ^Z under strace. And I used strace exactly because I wanted do debug some (I can't recall exactly) problems with jctl. That is all.

But in any case. Some users run the services under ptrace. I mean, the application borns/runs/dies under ptrace. That is why personally I certainly do not like anything which delays until detach (say, the-tracee-doesnt-participate-in-group-stop-until-detach logic).

Quoted 3 lines

If I change the patchset such that group stop while ptraced first enters TASK_STOPPED and then transitions into TASK_TRACED on the next PTRACE call,

Again, I am not sure I understand what exactly you mean... If you mean that it is wrong to simply change the state of the tracee in ptrace_check_attach() without arch_ptrace_stop() - I agree, this probably should be fixed.

I am wondering, if there is a simpler change... probably not.

But. this looks a bit off-topic (I mean, this looks orthogonal to the other things we are discussing), or I missed something else?

Quoted 1 line

there will be race window which would be visible

Personally, I think this is fine.

Hello, Oleg. Sorry about the delay.

Quoted 10 lines · Oleg Nesterov

Quoted 6 lines

Note that { the task is put into TASK_TRACED state and group stop resume by SIGCONT is ignored. | the task is put into TASK_STOPPED state and the following PTRACE request will transition it into TASK_TRACED. If SIGCONT is received before transition to TASK_TRACED is made, the task will resume execution. If PTRACE request faces with SIGCONT, PTRACE request may fail. }

To me, the first variant looks better. But, only because it is closer to the current behaviour. I mean, it is better to change the things incrementally.

Alright, it's the first variant then.

Quoted 4 lines

But in the longer term - I do not know. Personally, I like the TASK_STOPPED variant. To the point, I was thinking that (perhaps) we can change ptrace_stop() so that it simply calls do_signal_stop() if it notices ->group_stop_count != 0.

I don't know. IMHO it's enough to give the ptracer a way to honor group stop and notification so things like strace can stay more or less transparent. I don't think it's something we need to enforce.

Quoted 2 lines

And this is what I do not like. I just can't accept the fact there is a running thread in the SIGNAL_STOP_STOPPED group.

Let's agree to disagree here. I agree it's weird but don't think it's necessarily a bad thing that needs to be changed.

Quoted 1 line

Yes! and this is very good argument in favour of all your objections ;)

My objections to what? The one thing that I'm really against is allowing group stop to override PTRACE_CONT and that's visible regardless of notifications. Other than that, I think we're pretty much in agreement now, aren't we?

Quoted 5 lines

Oh, currently I am ignoring this, my only concern is how this all looks to the userland. But this is the good point, and I have to admit that I never realized this is just wrong. Yes, I agree, we should do something, but this is not visible to user-space (except this should fix the bug ;)

Mostly not but there's the obscure window where the tracee goes through TASK_RUNNING which can be visible from userland from another task of the ptracer group, which I think is ignorable as long as it's properly masked from the ptracing task itself. I wanted to make sure we agree on that one.

Quoted 2 lines

Sure, if we are talking about SIGCONT from "nowhere". But, the same way ^Z is not reliable too.

It doesn't even have to be from "nowhere". A process can be raising SIGCONT itself. To me, the whole thing feels more like an administration aid by design than a strict monitor/control mechanism.

Quoted 8 lines

I hate this from the time when I noticed that the application doesn't respond to ^Z under strace. And I used strace exactly because I wanted do debug some (I can't recall exactly) problems with jctl. That is all.

But in any case. Some users run the services under ptrace. I mean, the application borns/runs/dies under ptrace. That is why personally I certainly do not like anything which delays until detach (say, the-tracee-doesnt-participate-in-group-stop-until-detach logic).

I see, so let's make them participate in the group stop and notify the real parent of the group stop.

As long as PTRACE_CONT behavior isn't
changed, I don't object.

Hello, Oleg.

Quoted 4 lines · Oleg Nesterov

Quoted 2 lines

I don't know. Maybe it's more consistent that way and I'm not fundamentally against that but it is a big behavior change

Hmm. I tried to describe the current behaviour...

We can make it behave like the following. { | } denotes two alternative behaviors regarding SIGCONT.

If a group stop is initiated while, or was in progress when a task is ptraced, the task will stop for group stop and notify the ptracer accordingly. Note that the task could be trapped elsewhere delaying this from happening. When the task stops for group stop, it participates in group stop as if it is not ptraced and the real parent is notified of group stop completion.

Note that { the task is put into TASK_TRACED state and group stop resume by SIGCONT is ignored. | the task is put into TASK_STOPPED state and the following PTRACE request will transition it into TASK_TRACED. If SIGCONT is received before transition to TASK_TRACED is made, the task will resume execution. If PTRACE request faces with SIGCONT, PTRACE request may fail. }

The ptracer may resume execution of the task using PTRACE_CONT without affecting other tasks in the group. The task will not stop for the same group stop again while ptraced.

On ptrace detach, if group stop is in effect, the task will be put into TASK_STOPPED state and if it is the first time the task is stopping for the current group stop, it will participate in group stop completion.

This can be phrased better but it seems well defined enough for me. I take it that one of your concerns is direct transition into TASK_TRACED on group stop while ptraced which prevents the tracee from reacting to the following SIGCONT. I'm not sure how much of an actual problem it is given that our notification to real parent hasn't worked at all till now but we can definitely implement proper TASK_STOPPED -> TRACED transition on the next PTRACE request. There exists a fundamental race condition between SIGCONT and the next PTRACE call but I don't think it's such a big deal as long as the transition itself is done properly.

If we don't go that route, another solution would be to add a ptrace call which can listen to SIGCONT. ie. PTRACE_WAIT_CONT or whatever which the ptracer can call once it knows the tracee entered group stop.

In either case, the fundamentals of ptrace operation don't really change. All ptrace operations are still per-task and ptracer almost always has control over execution of the tracee. Sure, it allows ptraced task to escape group stop but it seems defined clear enough and IMHO actually is a helpful debugging feature. After all, it's not like stop signals can be used for absoultely reliable job control. There's an inherent race against SIGCONT.

Quoted 7 lines

Quoted 2 lines

What do you do about PTRACE requests while a task is group stopped? Reject them? Block them?

Yes, another known oddity. Of course we shouldn't reject or block. Perhaps we can ignore this initially. If SIGCONT comes after another request does STOPPED/TRACED it clears SIGNAL_STOP_STOPPED, but the tracee won't run until the next PTRACE_CONT, this makes sense.

That conceptually might make sense but other than the conceptual integrity it widely changes the assumptions and is less useful than the current behavior. I don't really see why we would want to do that.

Quoted 8 lines

The problem is, gdb can't leave the tracee in STOPPED state if it wants. We need to improve this somehow (like in your previous example with gdb).

Only if it attaches to every thread in the thread group. Otherwise, if the non-thread has already initiated the group-stop, the tracee will notice TIF_SIGPENDING eventually and call do_signal_stop(), debugger can't control this.

The debugger is still notified and can override it. gdb already can and does.

Quoted 6 lines

Quoted 5 lines

I don't think it's an extreme corner case
we can break. For example, if a user gdb's a program which raises one of the stop signals, currently the user expects to be able to continue the program from whithin the gdb. If we make group stop override ptrace, there's no other recourse than sending signal from outside.

Quoted 6 lines

Yes. Of course, gdb can be "fixed", it can send SIGCONT.

But yes, this is the noticeable change, that is why I suggested ptrace_resume-acts-as-SIGCONT logic. Ugly, yes, but more or less compatible. (although let me repeat, _pesonally_ I'd prefer to simply tell user-space to learn the new rules ;)

I can't really agree there. First, to me, it seems like too radical a change and secondly the resulting behavior might look conceptually pleasing but is not as useful as the current one. Why make a change which results in reduced usefulness while noticeably breaking existing users?

Quoted 6 lines

Quoted 2 lines

The only thing it achieves is the integrity of group stop

Given that SIGCHLD doesn't queue and with or without your changes we send it per-thread, it is not trivial for gdb to detect the group-stop anyway. Again, the kernel should help somehow.

Hmmm? Isn't this discoverable from the exit code from wait?

Quoted 4 lines

Quoted 3 lines

and I'm not really sure whether that's something worth achieving at the cost of debugging capabilities especially when we don't _have_ to lose them.

Quoted 2 lines

But we do not? I mean, at least this is not worse than the current behaviour.

I think it's worse. With your changes, debuggers can't diddle the tasks behind group stop's back which the current users already expect.

But this "diddling behind group stop's back" is exactly the current problem with stop signals.

Here I try to stop a ptraced process:

$ strace -tt sleep 30
23:02:15.619262 execve("/bin/sleep", ["sleep", "30"], [/* 30 vars */]) = 0
...
23:02:15.622112 nanosleep({30, 0}, NULL) = ? ERESTART_RESTARTBLOCK (To be restarted)
23:02:23.781165 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
23:02:23.781251 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
    (I forgot again why we see it twice. Another quirk I guess...)
23:02:23.781310 restart_syscall(<... resuming interrupted call ...>) = 0
23:02:45.622433 close(1)                = 0
23:02:45.622743 close(2)                = 0
23:02:45.622885 exit_group(0)           = ?

Why sleep didn't stop?

Because PTRACE_SYSCALL brought the task out of group stop at once, even though strace did try hard to not do so:

    ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP) <-- note SIGSTOP!

PTRACE_CONT in this situation would do the same.

You are saying that it is useful that gdb restarts group-stopped task with mere PTRACE_CONT. Above is a counter-example where it is anti-useful:
I would muchly prefer strace to see task sit stopped until it gets SIGCONT (or some fatal signal).

Why gdb can't use SIGCONT instead of PTRACE_CONT, just like every other tool which needs to resume stopped tasks?

Hello, Denys.

Quoted 17 lines · Denys Vlasenko

$ strace -tt sleep 30
23:02:15.619262 execve("/bin/sleep", ["sleep", "30"], [/* 30 vars */]) = 0
...
23:02:15.622112 nanosleep({30, 0}, NULL) = ? ERESTART_RESTARTBLOCK (To be restarted)
23:02:23.781165 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
23:02:23.781251 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
    (I forgot again why we see it twice. Another quirk I guess...)
23:02:23.781310 restart_syscall(<... resuming interrupted call ...>) = 0
23:02:45.622433 close(1)                = 0
23:02:45.622743 close(2)                = 0
23:02:45.622885 exit_group(0)           = ?

Why sleep didn't stop?

Because PTRACE_SYSCALL brought the task out of group stop at once, even though strace did try hard to not do so:

Quoted 3 lines

    ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP) <-- note SIGSTOP!

PTRACE_CONT in this situation would do the same.

This can be fixed by updating strace, right? strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

Quoted 2 lines

Why gdb can't use SIGCONT instead of PTRACE_CONT, just like every other tool which needs to resume stopped tasks?

Because that's how PTRACE_CONT behaved the whole time.

Quoted 5 lines

Quoted 1 line

PTRACE_CONT in this situation would do the same.

This can be fixed by updating strace, right? strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

Yes, in this particular case strace could be more clever.

But. The tracee should react to SIGCONT after that, this means we shouldn't "delay" this stop or force the TASK_TRACED state.

And note that in this case real_parent == debugger. Another case is more interesting, and this means we shouldn't delay or hide the notifications.

(I just tried to summarize the previous discussion for Denys)

Quoted 6 lines · Oleg Nesterov

Quoted 5 lines

Quoted 1 line

PTRACE_CONT in this situation would do the same.

This can be fixed by updating strace, right? strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

Quoted 4 lines

Yes, in this particular case strace could be more clever.

But. The tracee should react to SIGCONT after that, this means we shouldn't "delay" this stop or force the TASK_TRACED state.

Yeap, which is achievable by treating group stop differently from ptrace traps and make it proceed to TASK_TRACED only if ptrace wants to issue commands.

(reiterating just to make sure there's no
misunderstanding)

Hello, Denys.

Quoted 2 lines · Denys Vlasenko

But this "diddling behind group stop's back" is exactly the current problem with stop signals.

Maybe.  I don't necessarily agree but can see your point too but I think more important part is that that's a behavior which is quite noticeable from userland.

Quoted 19 lines

Here I try to stop a ptraced process:

$ strace -tt sleep 30
23:02:15.619262 execve("/bin/sleep", ["sleep", "30"], [/* 30 vars */]) = 0
...
23:02:15.622112 nanosleep({30, 0}, NULL) = ? ERESTART_RESTARTBLOCK (To be restarted)
23:02:23.781165 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
23:02:23.781251 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
    (I forgot again why we see it twice. Another quirk I guess...)
23:02:23.781310 restart_syscall(<... resuming interrupted call ...>) = 0
23:02:45.622433 close(1)                = 0
23:02:45.622743 close(2)                = 0
23:02:45.622885 exit_group(0)           = ?

Why sleep didn't stop?

Because PTRACE_SYSCALL brought the task out of group stop at once, even though strace did try hard to not do so:

Quoted 3 lines

    ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP) <-- note SIGSTOP!

PTRACE_CONT in this situation would do the same.

This can be fixed by updating strace, right?  strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

Quoted 4 lines

You are saying that it is useful that gdb restarts group-stopped task with mere PTRACE_CONT. Above is a counter-example where it is anti-useful:
I would muchly prefer strace to see task sit stopped until it gets SIGCONT (or some fatal signal).

This is more of an issue which can be improved in strace.  Sure, changing the kernel to enforce group stop over ptrace would make this case behave better but at the cost of breaking gdb.

Quoted 2 lines

Why gdb can't use SIGCONT instead of PTRACE_CONT, just like every other tool which needs to resume stopped tasks?

Because that's how PTRACE_CONT behaved the whole time.  It can but just hasn't needed to.

Quoted 8 lines

Quoted 8 lines

23:02:15.622112 nanosleep({30, 0}, NULL) = ? ERESTART_RESTARTBLOCK (To be restarted) 23:02:23.781165 --- SIGSTOP (Stopped (signal)) @ 0 (0) --- 23:02:23.781251 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
    (I forgot again why we see it twice. Another quirk I guess...) 23:02:23.781310 restart_syscall(<... resuming interrupted call ...>) = 0 23:02:45.622433 close(1)                = 0
23:02:45.622743 close(2)                = 0
23:02:45.622885 exit_group(0)           = ?

Quoted 3 lines

This can be fixed by updating strace, right?  strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

But tracee didn't stop _yet_. Signal is not delivered _yet_, debugger can decide at this point whether to deliver it:
ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP)
or ignore:
ptrace(PTRACE_SYSCALL, $PID, 0x1, 0)

strace has to deliver SIGSTOP if it wants to make program run exactly as it would run without strace. So it tries to do so.
Currently, ptrace machinery doesn't react as strace, its user, expects it to.

Quoted 8 lines · Denys Vlasenko

Quoted 8 lines

Quoted 8 lines

23:02:15.622112 nanosleep({30, 0}, NULL) = ? ERESTART_RESTARTBLOCK (To be restarted) 23:02:23.781165 --- SIGSTOP (Stopped (signal)) @ 0 (0) --- 23:02:23.781251 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
    (I forgot again why we see it twice. Another quirk I guess...) 23:02:23.781310 restart_syscall(<... resuming interrupted call ...>) = 0 23:02:45.622433 close(1)                = 0
23:02:45.622743 close(2)                = 0
23:02:45.622885 exit_group(0)           = ?

...

Quoted 10 lines

Quoted 3 lines

This can be fixed by updating strace, right?  strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

But tracee didn't stop _yet_. Signal is not delivered _yet_, debugger can decide at this point whether to deliver it:
ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP)
or ignore:
ptrace(PTRACE_SYSCALL, $PID, 0x1, 0)

Quoted 3 lines

strace has to deliver SIGSTOP if it wants to make program run exactly as it would run without strace. So it tries to do so.
Currently, ptrace machinery doesn't react as strace, its user, expects it to.

Okay, maybe I'm missing something but so once SIGSTOP is determined to be delivered, then the tracee enters group stop and that's the second SIGSTOP notification you get.

At that point, strace should wait for
the tracee to be continued by SIGCONT. That should work, right?

Hello,

Quoted 8 lines · Denys Vlasenko

Quoted 8 lines

Quoted 8 lines

23:02:15.622112 nanosleep({30, 0}, NULL) = ? ERESTART_RESTARTBLOCK (To be restarted) 23:02:23.781165 --- SIGSTOP (Stopped (signal)) @ 0 (0) --- 23:02:23.781251 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
    (I forgot again why we see it twice. Another quirk I guess...) 23:02:23.781310 restart_syscall(<... resuming interrupted call ...>) = 0 23:02:45.622433 close(1)                = 0
23:02:45.622743 close(2)                = 0
23:02:45.622885 exit_group(0)           = ?

...

Quoted 10 lines

Quoted 3 lines

This can be fixed by updating strace, right?  strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

But tracee didn't stop _yet_. Signal is not delivered _yet_, debugger can decide at this point whether to deliver it:
ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP)
or ignore:
ptrace(PTRACE_SYSCALL, $PID, 0x1, 0)

Quoted 3 lines

strace has to deliver SIGSTOP if it wants to make program run exactly as it would run without strace. So it tries to do so.
Currently, ptrace machinery doesn't react as strace, its user, expects it to.

Okay, maybe I'm missing something but so once SIGSTOP is determined to be delivered, then the tracee enters group stop and that's the second SIGSTOP notification you get.  At that point, strace should wait for the tracee to be continued by SIGCONT.  That should work, right?

Quoted 4 lines

Okay, maybe I'm missing something but so once SIGSTOP is determined to be delivered, then the tracee enters group stop and that's the second SIGSTOP notification you get.  At that point, strace should wait for the tracee to be continued by SIGCONT.  That should work, right?

Do you mean "Will it work on current kernels" or "that's what strace has to do and then it is supposed to work correctly, modulo bugs"?

In this particular scenario, first SIGSTOP is ptrace-stop. Obviously, we must issue ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP) to continue.

Second SIGSTOP is notification of tracee's group-stop to debugger.

The question is, logically, by sending this notification, does tracee, or does it not enter into ptrace-stop too? (IOW: is ptrace-stop a separate bit in task state, independent of group-stop?)
If yes, then we need to release tracee from ptrace-stop (but it will remain in group-stop) by issuing ptrace(PTRACE_SYSCALL, $PID, 0x1, 0).
If not, then we must not do so, because the task is not ptrace-stopped, and ptrace(PTRACE_SYSCALL, $PID, 0x1, 0) is undefined (I think it should error out to indicate that).

1. The distinction between the first SIGSTOP trapping and the second can only be reliably done by GETSIGINFO which in turn will put the tracee into TASK_TRACED making the tracee ignore the future SIGCONT

2. Due to reparenting, wait(2) notifications (including the SIGCLDs) don't get to the real parent at all.

#2 just needs fixing.

That preciesly is what is being discussed. IIUC, Oleg and Roland are saying that the tracee should enter group stop but not ptrace trap at that point and then transition into ptrace trap on the first PTRACE call.

Quoted 5 lines · Tejun Heo

That preciesly is what is being discussed. IIUC, Oleg and Roland are saying that the tracee should enter group stop but not ptrace trap at that point and then transition into ptrace trap on the first PTRACE call.

Actually I am not saying this (at least now, probably I did).

Once again. We have the bug with arch_ptrace_stop_needed(), but lets ignore it to simplify the discussion.

Suppose that the tracee calls do_signal_stop() and participates in the group stop. To me, it doesn't really matter (in the context of this discussion) if it stops in TASK_STOPPED or TASK_TRACED (and where it stops).

However, I am starting to agree that TASK_TRACED looks more clean.

What is important, I think ptrace should respect SIGNAL_STOP_STOPPED. IOW, when the tracee is group-stopped (TASK_STOPPED or TASK_TRACED, doesn't matter), ptrace_resume() should not wake it up, but merely do set_task_state(TASK_STATE) and make it resumeable by SIGCONT.

However, I am starting to agree that TASK_TRACED looks more clean.

What is important, I think ptrace should respect SIGNAL_STOP_STOPPED. IOW, when the tracee is group-stopped (TASK_STOPPED or TASK_TRACED, doesn't matter), ptrace_resume() should not wake it up, but merely do set_task_state(TASK_STATE) and make it resumeable by SIGCONT.

Okay, I think I finally caught up with the discussion (hopefully).

Quoted 2 lines · Oleg Nesterov

However, I am starting to agree that TASK_TRACED looks more clean.

Quoted 4 lines

What is important, I think ptrace should respect SIGNAL_STOP_STOPPED. IOW, when the tracee is group-stopped (TASK_STOPPED or TASK_TRACED, doesn't matter), ptrace_resume() should not wake it up, but merely do set_task_state(TASK_STATE) and make it resumeable by SIGCONT.

I don't think that's gonna fly. It first is a very user-visible change to how ptrace_resume() works

and it removes a lot of debugging
capability.

As Jan's examples showed, there are things which the debugger does behind group stop's back and some of them are quite legitimate and useful things to do like running some code

If you mix ptrace trap and group stop and then fix group stop notification, not only multithreaded debugging becomes quite cumbersome (suddenly ptracing becomes per-process thing instead of per-thread),

it becomes almost impossible to debug jctl behaviors.
Jctl becomes completely intertwined with ptracing and the real parent would get numerous notifications during the course of debugging.

I think they belong to different layers and they should stack instead of mix. I'll try to write up a summary for how I think it can be done later

but in short I think we just need two more PTRACE calls (one for combined SIGSTOPless attach + INTERRUPT

and the other for jctl
monitoring)

and there doesn't need to be any fundamental revolt in how ptrace and jctl interact with each other. The current ptrace behavior is quirky and rough on the edges but I think the fundamentals are correct in that it's something which is fundamentally bound to a task (not task group)

That way, most current users won't notice (e.g. entering TASK_TRACED directly on SIGSTOP doesn't make any different to strace or gdb,

Quoted 5 lines · Tejun Heo

Quoted 4 lines

What is important, I think ptrace should respect SIGNAL_STOP_STOPPED. IOW, when the tracee is group-stopped (TASK_STOPPED or TASK_TRACED, doesn't matter), ptrace_resume() should not wake it up, but merely do set_task_state(TASK_STATE) and make it resumeable by SIGCONT.

Quoted 2 lines

I don't think that's gonna fly. It first is a very user-visible change to how ptrace_resume() works

Yes. But can't resist, this is a bit unfair ;) It was you who convinced me we should cleanup this horror somehow, even if we break some corner cases.

However, again, I can't argue. Perhaps this change is too radical.

Quoted 3 lines

As Jan's examples showed, there are things which the debugger does behind group stop's back and some of them are quite legitimate and useful things to do like running some code

Yes. This can surprise a user which runs the unmodified debugger.

Quoted 4 lines

If you mix ptrace trap and group stop and then fix group stop notification, not only multithreaded debugging becomes quite cumbersome (suddenly ptracing becomes per-process thing instead of per-thread),

It should be, imho. Like SIGKILL, SIGSTOP/SIGCONT are not per-thread. This is per-process thing.

Quoted 3 lines

it becomes almost impossible to debug jctl behaviors.
Jctl becomes completely intertwined with ptracing and the real parent would get numerous notifications during the course of debugging.

Again, I think this is a win. The real parent should know that, say, its child becomes running after it was stopped. It does not matter why it was CLD_CONTINUED, it was resumed and that is all.

Quoted 3 lines

I think they belong to different layers and they should stack instead of mix. I'll try to write up a summary for how I think it can be done later

OK. You know, we already spent sooooooooooooooooooooooooooooooooooooooo much time discussing this, I have the strong desire to agree in advance with anything new ;)

Quoted 2 lines

but in short I think we just need two more PTRACE calls (one for combined SIGSTOPless attach + INTERRUPT

Yes, we are discussing these requests on archer,

Quoted 1 line

and the other for jctl monitoring)

Of course, we can add the new requests to help gdb/strace/whatever to handle jctl. In fact I think we should in any case.

But this is "easy". In the context of this discussion, my only concern is the current behaviour.

Quoted 1 line

Yes, we are discussing these requests on archer,

Can we please do that on LKML? It's a kernel change after all.

Hello,

Quoted 5 lines · Oleg Nesterov

Quoted 4 lines

If you mix ptrace trap and group stop and then fix group stop notification, not only multithreaded debugging becomes quite cumbersome (suddenly ptracing becomes per-process thing instead of per-thread),

Quoted 2 lines

It should be, imho. Like SIGKILL, SIGSTOP/SIGCONT are not per-thread. This is per-process thing.

jctl should be and will stay to be per-process, but that doesn't mean ptrace needs to interact with them at process level. ptrace can still be per-task and operate beneath jctl, which is what I'm proposing to do.

Quoted 4 lines

Quoted 2 lines

but in short I think we just need two more PTRACE calls (one for combined SIGSTOPless attach + INTERRUPT

Yes, we are discussing these requests on archer,

Can we please do that on LKML? It's a kernel change after all.

Quoted 18 lines

Quoted 18 lines

$ strace -tt sleep 30
23:02:15.619262 execve("/bin/sleep", ["sleep", "30"], [/* 30 vars */]) = 0
...
23:02:15.622112 nanosleep({30, 0}, NULL) = ? ERESTART_RESTARTBLOCK (To be restarted)
23:02:23.781165 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
23:02:23.781251 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
    (I forgot again why we see it twice. Another quirk I guess...)
23:02:23.781310 restart_syscall(<... resuming interrupted call ...>) = 0
23:02:45.622433 close(1)                = 0
23:02:45.622743 close(2)                = 0
23:02:45.622885 exit_group(0)           = ?

Why sleep didn't stop?

Because PTRACE_SYSCALL brought the task out of group stop at once, even though strace did try hard to not do so:

Quoted 7 lines

Quoted 3 lines

    ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP) <-- note SIGSTOP!

PTRACE_CONT in this situation would do the same.

This can be fixed by updating strace, right?  strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

But tracee didn't stop _yet_.

Signal is not delivered _yet_, debugger
can decide at this point whether to deliver it:
ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP)
or ignore:
ptrace(PTRACE_SYSCALL, $PID, 0x1, 0)

strace has to deliver SIGSTOP if it wants to make program run exactly as it would run without strace. So it tries to do so.
Currently, ptrace machinery doesn't react as strace, its user, expects it to.

Quoted 5 lines

Quoted 2 lines

Why gdb can't use SIGCONT instead of PTRACE_CONT, just like every other tool which needs to resume stopped tasks?

Because that's how PTRACE_CONT behaved the whole time.  It can but just hasn't needed to.

Jan, please put on your gdb maintainer's hat, we need your opinion here. Is it a problem from gdb's POV?

Quoted 18 lines · Denys Vlasenko

Quoted 18 lines

Quoted 18 lines

$ strace -tt sleep 30
23:02:15.619262 execve("/bin/sleep", ["sleep", "30"], [/* 30 vars */]) = 0
...
23:02:15.622112 nanosleep({30, 0}, NULL) = ? ERESTART_RESTARTBLOCK (To be restarted)
23:02:23.781165 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
23:02:23.781251 --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
    (I forgot again why we see it twice. Another quirk I guess...)
23:02:23.781310 restart_syscall(<... resuming interrupted call ...>) = 0
23:02:45.622433 close(1)                = 0
23:02:45.622743 close(2)                = 0
23:02:45.622885 exit_group(0)           = ?

Why sleep didn't stop?

Because PTRACE_SYSCALL brought the task out of group stop at once, even though strace did try hard to not do so:

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

    ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP) <-- note SIGSTOP!

PTRACE_CONT in this situation would do the same.

This can be fixed by updating strace, right?  strace can look at the wait(2) exit code and if the tracee stopped for group stop, wait for the tracee to be continued instead of issuing PTRACE_SYSCALL.

Ah, I seem to understand the confusion, let me repeat...

Quoted 1 line

But tracee didn't stop _yet_.

This depends on "_yet_". strace does ptrace(SYSCALL, SIGSTOP) twice. The first time it does this after the tracee reports the signal, and the tracee stopps.

Quoted 9 lines

Signal is not delivered _yet_, debugger
can decide at this point whether to deliver it:
ptrace(PTRACE_SYSCALL, $PID, 0x1, SIGSTOP)
or ignore:
ptrace(PTRACE_SYSCALL, $PID, 0x1, 0)

strace has to deliver SIGSTOP if it wants to make program run exactly as it would run without strace. So it tries to do so.
Currently, ptrace machinery doesn't react as strace, its user, expects it to.

It does, see above. But then the tracee actually stopps, and report this to the tracer. However, strace handles this case as if this was another signal=SIGSTOP, so it does ptrace(SYSCALL, SIGSTOP) again.

SIGSTOP has no effect, but PTRACE_SYSCALL wakeups the tracee.

I tried to simply not do ptrace(PTRACE_SYSCALL, ..., 0) at all. Behavior changes, but it is still wrong. Now tracee doesn't wake up on SIGCONT.

19:41:09.601764 wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGSTOP}], __WALL, NULL) = 25339
19:41:09.601914 rt_sigprocmask(SIG_BLOCK, [HUP INT QUIT PIPE TERM], NULL, 8) = 0 19:41:09.602081 ptrace(PTRACE_GETSIGINFO, 25339, 0, {si_signo=SIGSTOP,

si_code=SI_USER, si_pid=10105, si_uid=0, si_value={int=0, ptr=0}}) = 0 19:41:09.602273 write(2, "--- SIGSTOP (Stopped (signal)) @ 0 (0) ---
STOP: si_signo:19 si_code:0 si_status:0 si_value:(nil) \n", 99) = 99 19:41:09.602456 ptrace(PTRACE_SYSCALL, 25339, 0x1, SIGSTOP) = 0 19:41:09.602582 --- SIGCHLD (Child exited) @ 0 (0) --- 19:41:09.602652 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0 19:41:09.602792 wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGSTOP}], __WALL, NULL) = 25339

19:41:09.603081 ptrace(PTRACE_GETSIGINFO, 25339, 0, 0x7fff436fc730) =

Quoted 4 lines · Denys Vlasenko

I tried to simply not do ptrace(PTRACE_SYSCALL, ..., 0) at all. Behavior changes, but it is still wrong. Now tracee doesn't wake up on SIGCONT.

please see below,

Quoted 4 lines

19:41:09.601764 wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGSTOP}], __WALL, NULL) = 25339
19:41:09.601914 rt_sigprocmask(SIG_BLOCK, [HUP INT QUIT PIPE TERM], NULL, 8) = 0 19:41:09.602081 ptrace(PTRACE_GETSIGINFO, 25339, 0, {si_signo=SIGSTOP,

Quoted 8 lines

si_code=SI_USER, si_pid=10105, si_uid=0, si_value={int=0, ptr=0}}) = 0 19:41:09.602273 write(2, "--- SIGSTOP (Stopped (signal)) @ 0 (0) ---
STOP: si_signo:19 si_code:0 si_status:0 si_value:(nil) \n", 99) = 99 19:41:09.602456 ptrace(PTRACE_SYSCALL, 25339, 0x1, SIGSTOP) = 0 19:41:09.602582 --- SIGCHLD (Child exited) @ 0 (0) --- 19:41:09.602652 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0 19:41:09.602792 wait4(-1, [{WIFSTOPPED(s) && WSTOPSIG(s) == SIGSTOP}], __WALL, NULL) = 25339

OK, it is stopped.

Quoted 1 line

19:41:09.603081 ptrace(PTRACE_GETSIGINFO, 25339, 0, 0x7fff436fc730) =

And this changes the state to TASK_TRACED. See another email from me. That is why SIGCONT doesn't work.

This is another problem, the kernel should help somehow. This was already discussed a bit, but it is not clear what exactly we can do.

Basically, we have TASK_RUNNING, TASK_STOPPED and TASK_TRACED states, and after entering TASK_TRACED state we lose information in which state we were before entering it. We need to remember old state and restore it in order for this example to work.

Or we can avoid entering TASK_TRACED on ptrace(PTRACE_GETSIGINFO) et al. Can we remain in TASK_STOPPED?

Quoted 2 lines

Or we can avoid entering TASK_TRACED on ptrace(PTRACE_GETSIGINFO) et al. Can we remain in TASK_STOPPED?

Oh, unlikely, I think.

Hello, Oleg, Denys.

Quoted 3 lines · Oleg Nesterov

Quoted 2 lines

Or we can avoid entering TASK_TRACED on ptrace(PTRACE_GETSIGINFO) et al. Can we remain in TASK_STOPPED?

Quoted 1 line

Oh, unlikely, I think.

Actually I was thinking along this line. We can allow PTRACE_GETSIGINFO to proceed without forcing the tracee into TRACED state, the rationale being the operation is required to tell between group stop and ptrace trap. Am I missing something?

Actually I was thinking along this line. We can allow PTRACE_GETSIGINFO to proceed without forcing the tracee into TRACED state, the rationale being the operation is required to tell between group stop and ptrace trap. Am I missing something?

Quoted 5 lines

Actually I was thinking along this line. We can allow PTRACE_GETSIGINFO to proceed without forcing the tracee into TRACED state, the rationale being the operation is required to tell between group stop and ptrace trap. Am I missing something?

The reason for the transition to TASK_TRACED is to prevent a race with SIGCONT waking the task. There is always a race with SIGKILL waking it, but the circumstances where that can really matter are far fewer. You need to make sure that the work PTRACE_GETSIGINFO does to access last_siginfo cannot race with that pointer disappearing or the stack space it points to becoming invalid. I think the use of siglock ensures that, but Oleg should verify it.

Quoted 7 lines

The reason for the transition to TASK_TRACED is to prevent a race with SIGCONT waking the task. There is always a race with SIGKILL waking it, but the circumstances where that can really matter are far fewer. You need to make sure that the work PTRACE_GETSIGINFO does to access last_siginfo cannot race with that pointer disappearing or the stack space it points to becoming invalid. I think the use of siglock ensures that, but Oleg should verify it.

Yes, I think this is safe.

I do not really like this idea because it looks a bit strange to treat PTRACE_GETSIGINFO specially, and this doesn't solve all problems. And, once again, I still hope we can change ptrace_resume() so that it doesn't wakeup the stopped (I mean, SIGNAL_STOP_STOPPED) tracee, in this case this hack is not needed.

And. We are going to add the new requests which doesn't need the stopped tracee anyway. So we can just add PTRACE_HAS_SIGINFO which returns child->last_siginfo != NULL. This looks simpler, and this is compatible. Of course this check is racy, but this doesn't matter. PTRACE_GETSIGINFO is equally racy if it doesn't change the state to TASK_TRACED.

Hello, Oleg.

Still trying to follow the new discussion.

Quoted 16 lines

Quoted 7 lines

The reason for the transition to TASK_TRACED is to prevent a race with SIGCONT waking the task. There is always a race with SIGKILL waking it, but the circumstances where that can really matter are far fewer. You need to make sure that the work PTRACE_GETSIGINFO does to access last_siginfo cannot race with that pointer disappearing or the stack space it points to becoming invalid. I think the use of siglock ensures that, but Oleg should verify it.

Yes, I think this is safe.

I do not really like this idea because it looks a bit strange to treat PTRACE_GETSIGINFO specially, and this doesn't solve all problems. And, once again, I still hope we can change ptrace_resume() so that it doesn't wakeup the stopped (I mean, SIGNAL_STOP_STOPPED) tracee, in this case this hack is not needed.

Quoted 5 lines

And. We are going to add the new requests which doesn't need the stopped tracee anyway. So we can just add PTRACE_HAS_SIGINFO which returns child->last_siginfo != NULL. This looks simpler, and this is compatible. Of course this check is racy, but this doesn't matter. PTRACE_GETSIGINFO is equally racy if it doesn't change the state to TASK_TRACED.

This is probably where we disagree the most but I think the weird part isn't making PTRACE_GETSIGINFO exempt from TASK_TRACE transition. The weirdness starts when the tracee is put into TASK_STOPPED while being ptraced. I think such dual modes of operation inherently lead to strange problems.

Instead of having simple "a ptracer stops in TASK_TRACED and its execution is under the control of ptrace",

The patch which puts the tracee into TASK_TRACED
on ATTACH already fix two problems discussed in this thread without doing anything wonky. I think it says a lot.

As it currently stands, SIGSTOP/CONT while ptraced doesn't work

and
even if we bend the rules subtly and provide sneaky ways like the above, userland needs to be modified to make use of it anyway.

I
think it would be far cleaner to simply make ptracee always stop in TASK_TRACED and give the ptracer a way to notice what's happening to the tracee

Quoted 1 line

Still trying to follow the new discussion.

And how it goes?

As for me, I am not sure I can follow it ;)

Quoted 2 lines

Instead of having simple "a ptracer stops in TASK_TRACED and its execution is under the control of ptrace",

In fact, I am not sure I really disagree with this part, but see below.

Quoted 3 lines

The patch which puts the tracee into TASK_TRACED
on ATTACH already fix two problems discussed in this thread without doing anything wonky. I think it says a lot.

Yes. One off-topice note... if we are talking about this patch only, I do not think it makes sense to add the new member into task_struct so that STOPPED/TRACED transition can always report the exactly correct ->exit_code. I think we can just use group_exit_code ?: SIGSTOP. But, again, this is off-topic.

Quoted 1 line

As it currently stands, SIGSTOP/CONT while ptraced doesn't work

And this is probably where we disagree the most. I think this is bug, and this should be fixed.

Quoted 2 lines

and even if we bend the rules subtly and provide sneaky ways like the above, userland needs to be modified to make use of it anyway.

Yes. But with the current code we can't modify, say, strace so that SIGSTOP/CONT can work "correctly".

Quoted 3 lines

I think it would be far cleaner to simply make ptracee always stop in TASK_TRACED and give the ptracer a way to notice what's happening to the tracee

Well. If we accept the proposed PTRACE_CONT-needs-SIGCONT behaviour, then I think this probably makes sense. The tracee stops under ptrace, the possible SIGCONT shouldn't abuse debugger which wants to know, say, the state of registers.

To be honest, I don't understand whether I changed my mind now, or I was never against this particular change in behaviour.

Once debugger does PTRACE_CONT, the tracee becomes TASK_STOPPED and now it is "visible" to SIGCONT (or the tracee resumes if SIGCONT has come in between).

But I think you will equally blame this TRACED/STOPPED transition as "behavioral subtleties" and I can understand you even if I disagree. And yes, this leads to other questions. But note that this greatly simplifies things. The tracee can never participate in the same group-stop twice.

BTW, I plan on separating out all ptrace related stuff into a separate struct as it's not used by most tasks anyway, so I don't think we need to be too concerned about several more fields.

Quoted 3 lines

BTW, I plan on separating out all ptrace related stuff into a separate struct as it's not used by most tasks anyway, so I don't think we need to be too concerned about several more fields.

This is funny.

I already did this change (it even had some review on lkml). And it was discussed again (on [email protected]) a couple of days ago. Yes, probably we should finally do this. Unfortunately, this complicates the attach-to-all-threads request (new feature), but anyway.

Hey,

Quoted 6 lines · Oleg Nesterov

Quoted 3 lines

BTW, I plan on separating out all ptrace related stuff into a separate struct as it's not used by most tasks anyway, so I don't think we need to be too concerned about several more fields.

This is funny.

Quoted 4 lines

I already did this change (it even had some review on lkml). And it was discussed again (on [email protected]) a couple of days ago. Yes, probably we should finally do this. Unfortunately, this complicates the attach-to-all-threads request (new feature), but anyway.

Heh, well, I also have a mostly working patch but hey if you already did it, all the better. :-)

Quoted 2 lines

Heh, well, I also have a mostly working patch but hey if you already did it, all the better. :-)

Argh, sorry. If you already have the patch - please send it.

Otherwise I'll try to refresh the old series, but not before the next week.

Quoted 5 lines

Quoted 2 lines

As it currently stands, SIGSTOP/CONT while ptraced doesn't work

And this is probably where we disagree the most. I think this is bug, and this should be fixed.

I don't think we disagree that it is a bug. I want to fix it too but we definitely seem to disagree on how.

I want to give more control to
the ptracer so that the tracer has enough information and control to follow the group stop semantics if it wants to and you want to give more control to group stop so that it overrides the tracer and always does the right thing regarding group stop.

Quoted 4 lines

Quoted 3 lines

I think it would be far cleaner to simply make ptracee always stop in TASK_TRACED and give the ptracer a way to notice what's happening to the tracee

Quoted 4 lines

Well. If we accept the proposed PTRACE_CONT-needs-SIGCONT behaviour, then I think this probably makes sense. The tracee stops under ptrace, the possible SIGCONT shouldn't abuse debugger which wants to know, say, the state of registers.

The objections I have against PTRACE_CONT-needs-SIGCONT are,

* It will be very different from the current behavior.

* ptrace, sans the odd SIGSTOP on attach which we should remove, is per-task. Sending out SIGCONT on PTRACE_CONT would break that. I really don't think that's a good idea.

* PTRACE_CONT would be behaving completely differently depending on whether it's resuming from group stop or other traps.

Quoted 4 lines

Once debugger does PTRACE_CONT, the tracee becomes TASK_STOPPED and now it is "visible" to SIGCONT (or the tracee resumes if SIGCONT has come in between).

Quoted 5 lines

But I think you will equally blame this TRACED/STOPPED transition as "behavioral subtleties" and I can understand you even if I disagree. And yes, this leads to other questions. But note that this greatly simplifies things. The tracee can never participate in the same group-stop twice.

But that's not really because the problem is solved. The problem is put out of scope by forcing the tracer to always override group stop.

Damn. Today is 02/24 ;) sorry.

Quoted 4 lines · Oleg Nesterov

Quoted 3 lines

Quoted 2 lines

As it currently stands, SIGSTOP/CONT while ptraced doesn't work

Quoted 5 lines

Quoted 2 lines

And this is probably where we disagree the most. I think this is bug, and this should be fixed.

I don't think we disagree that it is a bug. I want to fix it too but we definitely seem to disagree on how.

Yes, but I also think that the running tracee in the SIGNAL_STOP_STOPPED process is bug by itself. IIUC, you think this is fine.

Quoted 3 lines

* ptrace, sans the odd SIGSTOP on attach which we should remove, is per-task. Sending out SIGCONT on PTRACE_CONT would break that. I really don't think that's a good idea.

Hmm. But why do you think we should always send SIGCONT after attach?

Quoted 2 lines

* PTRACE_CONT would be behaving completely differently depending on whether it's resuming from group stop or other traps.

Afaics, no. It does not matter from where the tracee resumes. See the [pseudo patch] I sent. Once again, it doesn't really work, it only tries to explain what I mean.

Quoted 5 lines

Quoted 3 lines

* ptrace, sans the odd SIGSTOP on attach which we should remove, is per-task. Sending out SIGCONT on PTRACE_CONT would break that. I really don't think that's a good idea.

Hmm. But why do you think we should always send SIGCONT after attach?

Hmmm... my sentences were confusing. I was trying to say,

* ptrace, as it currently stands, is largely per-task. One exception is the implicit SIGSTOP which is sent on PTRACE_ATTACH but this should be replaced with a more transparent attach request which doesn't affect jctl states.

* Sending out SIGCONT on PTRACE_CONT on jctl stopped tracee adds another exception to per-task behavior, which I don't think is a good idea.

From my side, i really want to see "group-stop is ignored under ptrace"

* PTRACE_ATTACH's insertion of SIGSTOP is a design bug, but it is so ingrained by now that we don't want to change PTRACE_ATTACH semantic. We fix this situation by introducing a new ptrace call, PTRACE_ATTACH_NOSTOP, which has saner API.

* group-stop state is currently not preserved across ptrace-stop. This makes, in particular, ^Z and SIGSTOP inoperative for straced programs. Everyone agrees this needs to be fixed.
(There is a small bug of not notifying real parent about the group-stop, I don't want to go there since it is also non-contentious - everybody is in agreement this also should be fixed in "obvious" way).

* HOWEVER, this behavior _is_ indeed used by gdb to run small fragments of tracee even if it's stopped. Jan's example:

    # gdb -p applicationpid
    (gdb) print getpid()
    (gdb) print show_me_your_internal_debug_dump()
    (gdb) continue
  gdb people want to preserve this feature.
  How gdb implements this? I ssume it does this by modifying IP,
  setting a breakpoint on return address, and issues PTRACE_CONT(0).
  Currently it works because of "group-stop is ignored under ptrace" bug.

How we can accomodate this gdb need while fixing this bug?

Oleg's POV is that gdb should SIGCONT the tracee (at least if it is currently in group-stop). This has the advantage of using standard Unix tool. The disadvantage is that SIGCONT will wake up *all* threads, and that it will cause user-visible effects (SIGCONT handler will be run, parent can (or "should be able to", we may have a bug there too) see child to be WCONTINUED.

Frankly, it seems that this is hardly acceptable for gdb. gdb people do want here a "secret" backdoor-ish way to make a *thread* (not the whole process) running even when the process is in group-stop. Yes, this is a "violation" of the convention that normally stopped process has all threads stopped, and it makes Oleg feel it is "wrong", but it is also useful, and used in real life. We can't ignore that.

Jan's idea is to make kernel remember group-stop state upon attach, preserve current behavior of ignoring group-stop while attached, and restore group-stop upon detach.
Sorry Jan, this won't work in many cases. It won't fix the "stracing makes process ignore SIGSTOP" bug - the result will be that buggy behavior will be still observed. Neither it will work for

    # gdb -p applicationpid
    (gdb) print getpid()
    (gdb) print show_me_your_internal_debug_dump()
    (gdb) continue
- the "continue" will make application run even if we attached to it while
it was stopped. It will ONLY work for
    # gdb -p applicationpid
    (gdb) print getpid()
    (gdb) print show_me_your_internal_debug_dump()
    (gdb) quit
sequence. Which is good, but not good enough.

Tejun, you are disagreeng with Oleg's proposal.

Do you have a proposal which looks better to you? Or do you propose to just leave it as-is, that is, to continue to ignore group-stop under ptrace?

From my side, i really want to see "group-stop is ignored under ptrace" bug fixed, yet I feel gdb's needs are legitimate. Perhaps I can help by presenting a few ideas how to open a backdoor in ptrace API for gdb:

(a) Special-case ptrace(PTRACE_CONT/SYSCALL, pid, 0, SIGCONT) to do "special restart for gdb" thing. Problem with this idea is that we can be in ptrace-stop caused by genuine signal delivery, and using ptrace(PTRACE_CONT/SYSCALL, SIGCONT) from it means "inject SIGCONT".
IOW: this creates ambiquity.

or

(b) Abuse "addr" parameter in ptrace(PTRACE_CONT/SYSCALL, pid, addr, sig). Currently, it is unused. Can we define a value for it which means "do gdb hacky restart under group-stop, if tracee is indeed under group-stop"? (the value should be different from 0 and 0x1 - values currently used by strace)

or

(c) Add ptrace(PTRACE_CONT2/SYSCALL2/SINGLESTEP2) with the semantic of "do gdb hacky restart under group-stop, if tracee is indeed under group-stop". I like it less because we have at least three restarting PTRACE_foo, maybe even four if we want to have DETACH2 too.
Duplicating every one of them feels ugly.

or

(d) Add a ptrace option PTRACE_O_IGNORE_JOB_STOP which can be set/cleared by PTRACE_SETOPTIONS and which modifies ptrace-restart behavior. gdb will set the option before it wants to do "restart-which-ignores-group-stop", and clears it again when it no longer wants it. In the example above:

    # gdb -p applicationpid
    (gdb) print getpid() # sets IGNORE_JOB_STOP before PTRACE_CONT(0)
    (gdb) print show_me_your_internal_debug_dump() # sets IGNORE_JOB_STOP
    (gdb) continue       # clears IGNORE_JOB_STOP before PTRACE_CONT(0)

Quoted 6 lines

* group-stop state is currently not preserved across ptrace-stop. This makes, in particular, ^Z and SIGSTOP inoperative for straced programs. Everyone agrees this needs to be fixed.
  (There is a small bug of not notifying real parent about the group-stop, I don't want to go there since it is also non-contentious - everybody is in agreement this also should be fixed in "obvious" way).

Yeap, we do agree on this one, unfortunately not on how yet.

Quoted 10 lines

* HOWEVER, this behavior _is_ indeed used by gdb to run small fragments of tracee even if it's stopped. Jan's example:

    # gdb -p applicationpid
    (gdb) print getpid()
    (gdb) print show_me_your_internal_debug_dump()
    (gdb) continue
  gdb people want to preserve this feature.
  How gdb implements this? I ssume it does this by modifying IP,
  setting a breakpoint on return address, and issues PTRACE_CONT(0).
  Currently it works because of "group-stop is ignored under ptrace" bug.

I don't think it works because of "group-stop is ignored under ptrace" bug.

 IMO, it's because ptrace is inherently per-task not per-task-group, which I think is the right way to do it.

Yeah, agreed and as I said multiple times I think this is by design and actually the better and more useful behavior, albeit slightly less intuitive.

(gdb) print getpid()

gdb modifies IP, sets breakpoint on return address, and issues PTRACE_CONT(0). Kernel has to put the tracee into group-stop, right?
Becuase if it doesn't, if it makes tracee run, then the kernel is still broken. For example,
stracing a program and sending SIGSTOP on it won't work: the sequence of events will be
got SIGSTOP because SIGSTOP was delivered
PTRACE_SYSCALL(SIGSTOP) - "inject it"
got SIGSTOP because tracee is in group-stop now PTRACE_SYSCALL(SIGSTOP) - equivalent to PTRACE_SYSCALL(0)
because we aren't in signal delivery ptrace-stop
and tracee continues.

That's why I think gdb's "print getpid()" today depends on the bug. If we simply fix the bug (by making PTRACE_CONT/SYSCALL(0) re-enter group-stop), then "print getpid()" will stop working for stopped tracees.

Quoted 15 lines · Denys Vlasenko

(gdb) print getpid()

gdb modifies IP, sets breakpoint on return address, and issues PTRACE_CONT(0). Kernel has to put the tracee into group-stop, right?
Becuase if it doesn't, if it makes tracee run, then the kernel is still broken. For example,
stracing a program and sending SIGSTOP on it won't work: the sequence of events will be
got SIGSTOP because SIGSTOP was delivered
PTRACE_SYSCALL(SIGSTOP) - "inject it"
got SIGSTOP because tracee is in group-stop now PTRACE_SYSCALL(SIGSTOP) - equivalent to PTRACE_SYSCALL(0)
  because we aren't in signal delivery ptrace-stop
and tracee continues.

Quoted 4 lines

That's why I think gdb's "print getpid()" today depends on the bug. If we simply fix the bug (by making PTRACE_CONT/SYSCALL(0) re-enter group-stop), then "print getpid()" will stop working for stopped tracees.

There's no reason to make the tracee re-enter group stop after pulling it out to execute 'print getpid()'.

 The only thing necessary is a way
for the debugger to find out that group stop has been lifted.

The debugger then can resume the tracee if it wishes so.  ie. group stop becomes a trap point + a state which the debugger can monitor.  If the debugger wants the tracee to follow the jctl behavior, it can do so by resuming the tracee as it sees fit.

Quoted 2 lines

There's no reason to make the tracee re-enter group stop after pulling it out to execute 'print getpid()'.

If we want to execute 'print getpid()', you are right, we don't want to enter group stop. That's use case #1. But there is use case #2:
"strace sleep" + ^Z. if we want to continue stracing sleep without continuing, our PTRACE_SYSCALL(0) *must* make sleep enter group stop. Otherwise, sleep won't be stopped. It will continue sleeping and will exit, which is not what we want. Right?

Quoted 2 lines

 The only thing necessary is a way
for the debugger to find out that group stop has been lifted.

What do you mean by "has been *lifted*"?

Quoted 4 lines

The debugger then can resume the tracee if it wishes so.  ie. group stop becomes a trap point + a state which the debugger can monitor.  If the debugger wants the tracee to follow the jctl behavior, it can do so by resuming the tracee as it sees fit.

Can you describe this in more details? Do you propose that debugger can detect that we are in group stop (it is already sort-of possible with PTRACE_GETSIGINFO) and if it doesn't want to restart tracee, it simply doesn't do any PTRACE_SYSCALL/CONT? I tried that. This makes tracee sit in *ptrace* stop, not group stop.
Meaning: debugger is never be able to see waking SIGCONT:
waitpid doesn't report it to the debugger.

Hello,

Quoted 9 lines · Denys Vlasenko

Quoted 2 lines

There's no reason to make the tracee re-enter group stop after pulling it out to execute 'print getpid()'.

If we want to execute 'print getpid()', you are right, we don't want to enter group stop. That's use case #1. But there is use case #2:
"strace sleep" + ^Z. if we want to continue stracing sleep without continuing, our PTRACE_SYSCALL(0) *must* make sleep enter group stop. Otherwise, sleep won't be stopped. It will continue sleeping and will exit, which is not what we want. Right?