RFC: PTRACE_SEIZE needs API cleanup?

Hi guys,

I added code to use PTRACE_SEIZE in strace and in the process had tasted how API looks like from userspace POV.

It is usable, but API feels somewhat quirky.

Consider the following: one of reasons why we added PTRACE_SEIZE is that existing ptrace API has unnecessary complications (quirks) such as SIGSTOP on attach, SIGTRAP after execve.

But whoever designed strace did not deliberately designed these quirks in, he thought it was a good, reasonable design. Only after a second, third, tenth look it became obvious in retrospect that some things are not exactly right.

Thankfully, quirks in new PTRACE_SEIZE code mostly have the nature of "unnecessarily invented entities" as opposed to problems in trying to use API in real world tasks, but I still think they are annoying enough to be looked at.

We already have a mechanism how to modify ptrace behavior: ptrace options. But now we introduce a different mechnism to do the same: by using SEIZE instead of ATTACH, we magically change some aspects of ptrace. In effect, SEIZE sets some options. And these "SEIZE options" can't be set or cleared by SETOPTIONS. This is stupid. Why can't we just add more options instead of inventing new entities?

Why we overloaded
SEIZE with two functions: "attach to, but don't SIGSTOP the tracee" and "change behaviour of ptrace on this tracee"?

If the argument is that we want to set options immediately at attach, then I completely agree: yes, we do! Moreover, we want to set some _ordinary_ options too, such as PTRACE_O_TRACEEXEC, and we can't do that even now, in improved API! It needs more improving.

So my proposal is:
(a) make SEIZE take a parameter "immediately set these options on attach"

(b) without any options, make SEIZE just do "ATTACH sans SIGSTOP" thing, nothing more.
(c) make the following new PTRACE_O_foo options:
(1) "flag stops with PTRACE_EVENT_STOP event value in waitpid status"

(2) "enable PTRACE_INTERRUPT. It either causes PTRACE_EVENT_STOP with sig=SIGTRAP if (1) is enabled, or creates a group-stop with sig=SIGTRAP otherwise" [if the second part is too weird to implement, make (2) require (1)]

(3) "enable PTRACE_LISTEN. Works on group-stops even without any other options"

(4) "make auto-attached children stop a-la INTERRUPT, not with SIGSTOP"

(5) "enable saner error codes"

(d) Remove magic hidden 'I was SEIZEd, not ATTACHed' bit in kernel code.

    Use bits 1,2,3,4 instead of it.

(e) Make these bits settable with SETOPTIONS too, to keep API complete.

In strace, I already implicitly use 1,2,3,4. With this proposal, I'd use all five, *explicitly*, and also use at least PTRACE_O_TRACEEXEC, to avoid the possibility SEIZE races with execve and we get that pesky SIGTRAP. Something along the lines:

	ptrace(PTRACE_SEIZE, pid, 0,
		PTRACE_O_TRACEEXEC
		| PTRACE_O_TRACESTOP      /* (1) */
		| PTRACE_O_TRACEINTERRUPT /* (2) */
		| PTRACE_O_TRACELISTEN    /* (3) */
		| PTRACE_O_TRACECHILDSTOP /* (4) */
		| PTRACE_O_TRACEERRCODES  /* (5) */
	);

This will remove the need to set options on first ptrace stop.

This will remove the asymmetry we just introduced in current SEIZE code:
PTRACE_EVENT_STOP is the only event code which has no corresponding PTRACE_O_TRACESTOP option - situation which Occam disapproves.

In fact, I am unsure why the new ops (points 2 and 3 above) need enabling. Maybe they can be simply be always allowed? Let's see...

Re PTRACE_INTERRUPT: "old-style" check for group-stop, namely, "if ptrace(PTRACE_GETSIGINFO) fails, then it's group-stop",

works just fine, therefore, from userspace POV there is no apparent reason why PTRACE_INTERRUPT can't work without "flag stops with PTRACE_EVENT_STOP"
thingy: in this case it can create an
"WIFSTOPPED(status) = true, ptrace(PTRACE_GETSIGINFO) fails"
event which is detectable in userspace. PTRACE_EVENT_STOP is not needed for this, it's just a "nice to have" thing (it saves one ptrace call).

Re PTRACE_LISTEN: similarly to above, we _do_ know when we are in group-stop, even without (1) "flag stops with PTRACE_EVENT_STOP" thingy. Therefore we do know when PTRACE_LISTEN makes sense.

Moreover, we can make it so that kernel errors out if PTRACE_LISTEN is used in a ptrace stop of a wrong kind.

Just to let you see the relevant userspace code which made me think about the above issues, here is abridged version of main waitpid loop in strace. Note that it is coded in a way that it works both on SEIZE-enabled kernels and on old kernels, which have no SEIZE and friends. "use_seize" variable is set if we are on a SEIZE-enabled kernel:

	while (nprocs != 0) {
		pid = waitpid(-1, &status, __WALL);
		if (pid < 0) ...

		/* Look up 'pid' in our table of known tracees */
		tcp = pid2tcb(pid);
		if (tcp == NULL) {  /* Not found. It's auto-attached child */
			tcp = alloctcb(pid);
// post_attach_sigstop is == 0 if use_seize, or == TCB_IGNORE_ONE_SIGSTOP otherwise
// Basically, it's "do we expect to filter out one SIGSTOP"?
			tcp->flags |= TCB_ATTACHED | TCB_STARTUP | post_attach_sigstop;
		}

		if (WIFSIGNALED(status)) {

			...
			continue;
		}
		if (WIFEXITED(status)) {
			...
			continue;
		}
		if (!WIFSTOPPED(status)) ... /* error, should not happen */

		/* Is this the very first time we see this tracee stopped? */
		if (tcp->flags & TCB_STARTUP) {
			tcp->flags &= ~TCB_STARTUP;
			if (ptrace(PTRACE_SETOPTIONS, tcp->pid, NULL, ptrace_setoptions) < 0)
				if (errno != ESRCH)
					perror_msg_and_die("PTRACE_SETOPTIONS");
		}

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this if() will not be needed if we set options on attach!

		sig = WSTOPSIG(status);

		if (((unsigned)status >> 16) != 0) { /* Ptrace event */
			unsigned ev = (unsigned)status >> 16;
			if (ev == PTRACE_EVENT_STOP && sig != SIGTRAP) {
					stopped = 1;
					goto show_stopsig;
				}
			}
			goto restart_tracee_with_sig_0;
		}

		/* Is this post-attach SIGSTOP?
		 * Interestingly, the process may stop
		 * with STOPSIG equal to some other signal
		 * than SIGSTOP if we happend to attach
		 * just before the process takes a signal.
		 */
		if (sig == SIGSTOP && (tcp->flags & TCB_IGNORE_ONE_SIGSTOP)) {
			tcp->flags &= ~TCB_IGNORE_ONE_SIGSTOP;
			goto restart_tracee_with_sig_0;
		}

		if (sig != syscall_trap_sig) {
			siginfo_t si;

			/* Nonzero (true) if tracee is stopped by signal
			 * (as opposed to "tracee received signal") */
			stopped = ptrace(PTRACE_GETSIGINFO, pid, 0, &si);

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ here is how we determine whether it is group-stop or signal-delivery stop for the "we didn't use SEIZE" case (if we used SEIZE, we would not come here on group-stop: see ev == PTRACE_EVENT_STOP check and "goto show_stopsig" above).

show_stopsig:

			if (cflag != CFLAG_ONLY_STATS && (qual_flags[sig] & QUAL_SIGNAL)) {
				printleader(tcp);
				if (!stopped) {
					tprints("--- ");
					printsiginfo(&si, verbose(tcp));
				} else
					tprintf("--- stopped by %s ---", signame(sig));
				printtrailer();
			}

...and here we decide to use either PTRACE_SYSCALL or PTRACE_LISTEN, _based on the value of "stopped" variable_. Here we PTRACE_LISTEN only if use_seize is true, but we use that flag only as indicator "this is a new kernel". It's easy to see that there are no intrisic reasons
why it can't also work on newer kernels, but after using old-style ATTACH.

vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv

			if (!stopped)  /* It's signal-delivery-stop. Inject the signal */
				goto restart_tracee;

			/* It's group-stop */
			if (use_seize) {
				/* This ends ptrace-stop, but does _not_ end group-stop */
				if (ptrace_restart(PTRACE_LISTEN, tcp, 0) < 0) ... /* err, doesn't happen */
				continue;
			}
			/* Old kernel, no support for PTRACE_LISTEN */
			goto restart_tracee;
		}

		/* This is syscall entry or exit */
		if (trace_syscall(tcp) < 0 && !tcp->ptrace_errno) {
			...drop this tracee, it died...
			continue;
		}

restart_tracee_with_sig_0:

		sig = 0;
 restart_tracee:
		if (ptrace_restart(PTRACE_SYSCALL, tcp, sig) < 0) ... /* err, doesn't happen */
	}

Quoted 6 lines

We already have a mechanism how to modify ptrace behavior: ptrace options. But now we introduce a different mechnism to do the same: by using SEIZE instead of ATTACH, we magically change some aspects of ptrace. In effect, SEIZE sets some options. And these "SEIZE options" can't be set or cleared by SETOPTIONS. This is stupid. Why can't we just add more options instead of inventing new entities?

Exactly my feeling!

Quoted 9 lines

Why we overloaded
SEIZE with two functions: "attach to, but don't SIGSTOP the tracee" and "change behaviour of ptrace on this tracee"?

If the argument is that we want to set options immediately at attach, then I completely agree: yes, we do! Moreover, we want to set some _ordinary_ options too, such as PTRACE_O_TRACEEXEC, and we can't do that even now, in improved API! It needs more improving.

Quoted 2 lines

So my proposal is:
(a) make SEIZE take a parameter "immediately set these options on attach"

I think that would be a great improvement.

Better than making SEIZE an option itself, which I wanted to propose, but never got to.

Quoted 4 lines

(b) without any options, make SEIZE just do "ATTACH sans SIGSTOP" thing, nothing more.
(c) make the following new PTRACE_O_foo options:
(1) "flag stops with PTRACE_EVENT_STOP event value in waitpid status"

Why would anyone use that flag?

Most users are unaware of the crazy subtle SIGSTOP notification madness. It is totally unnecessary, just fix waitid()
to honour the absence of the WUNTRACED flag for traced processes.

I can't
think of any useful scenario where calling waitid() with WUNTRACED set on a traced task makes much sense, as the same information is received via ptrace.

The other part of a simpler, less crazy ptrace is let PTRACE_CONT not automatically continue a SIGSTOPped process.

Quoted 3 lines

(2) "enable PTRACE_INTERRUPT. It either causes PTRACE_EVENT_STOP with sig=SIGTRAP if (1) is enabled, or creates a group-stop with sig=SIGTRAP otherwise" [if the second part is too weird to implement, make (2) require (1)]

I don't understand the need for PTRACE_INTERRUPT: Sending some signal, trapping, and PTRACE_CONT with zero "data" should have the same effect.

Or do you mean that
instead of SIGSTOP, SIGTRAP should be sent? If so, yes, that would be much saner.

Quoted 1 line

(3) "enable PTRACE_LISTEN. Works on group-stops even without any other options"

This is a symptom of the complexity I whined about above.

If ptrace has no influence on group stops then PTRACE_LISTEN is not needed at all. The problem seems to be that SIGSTOP state is the same as ptrace trapped state, or something like that. Group stops and ptrace stops should be totally orthogonal. Then crazy things like PTRACE_LISTEN are not needed.

Why not just untangle the two concepts? (Via PTRACE_O_SANE.)

Quoted 1 line

(5) "enable saner error codes"

Why do you need a new option for this? Ptrace error codes are close to useless, how can returning better error codes ever cause problems?

Quoted 2 lines

In fact, I am unsure why the new ops (points 2 and 3 above) need enabling. Maybe they can be simply be always allowed? Let's see...

My advise is to untangle all ptrace and group stop state from each other with one PTRACE_O_SANE option. That would replace options 1 to 4. This gives perhaps less choice, but considering how subtle it already is, I don't think that simplifying everything is a bad thing.

Quoted 2 lines

Re PTRACE_INTERRUPT: "old-style" check for group-stop, namely, "if ptrace(PTRACE_GETSIGINFO) fails, then it's group-stop",

BTW, no one is going to use and know this, except perhaps ten people in the whole world.

Everyone else just thinks that ptrace doesn't pass on SIGSTOP, as the manpage says, and that the double SIGSTOP event is just a quirk.

Quoted 1 line

if (!WIFSTOPPED(status)) ... /* error, should not happen */

It may happen if you're tracing a child process and are getting a normal SIGCHLD?

Quoted 5 lines

...and here we decide to use either PTRACE_SYSCALL or PTRACE_LISTEN, _based on the value of "stopped" variable_. Here we PTRACE_LISTEN only if use_seize is true, but we use that flag only as indicator "this is a new kernel". It's easy to see that there are no intrisic reasons
why it can't also work on newer kernels, but after using old-style ATTACH.

So you're emulating normal group stop behaviour in the tracer, adding band-aids that make that possible, instead of avoiding these problems by making ptrace totally transparent for group stop state? Wouldn't it all be much simpler if ptrace didn't mess with group stop state?

Quoted 7 lines · Indan Zupancic

Quoted 2 lines

So my proposal is:
(a) make SEIZE take a parameter "immediately set these options on attach"

I think that would be a great improvement.

Better than making SEIZE an option itself, which I wanted to propose, but never got to.

Can't do. In pre-existing ptrace API, options can be set only after attach. Therefore, it's not possible to set an option which affects ATTACH behavior.

Quoted 7 lines

Quoted 5 lines

(b) without any options, make SEIZE just do "ATTACH sans SIGSTOP" thing, nothing more.
(c) make the following new PTRACE_O_foo options:
(1) "flag stops with PTRACE_EVENT_STOP event value in waitpid status"

Why would anyone use that flag?

Well, it's nice to not require GETSIGINFO in order to know whether this stop is group-stop.

Quoted 3 lines

Most users are unaware of the crazy subtle SIGSTOP notification madness. It is totally unnecessary, just fix waitid()
to honour the absence of the WUNTRACED flag for traced processes.

Can't do. Existing programs depend on current ptrace behavior.

Quoted 4 lines

I can't
think of any useful scenario where calling waitid() with WUNTRACED set on a traced task makes much sense, as the same information is received via ptrace.

Can't do. Existing programs depend on current ptrace behavior.

Quoted 3 lines

The other part of a simpler, less crazy ptrace is let PTRACE_CONT not automatically continue a SIGSTOPped process.

Can't do. gdb depends on current ptrace behavior that PTRACE_CONT restarts tracee after SIGSTOP.

Quoted 7 lines

Quoted 4 lines

(2) "enable PTRACE_INTERRUPT. It either causes PTRACE_EVENT_STOP with sig=SIGTRAP if (1) is enabled, or creates a group-stop with sig=SIGTRAP otherwise" [if the second part is too weird to implement, make (2) require (1)]

I don't understand the need for PTRACE_INTERRUPT: Sending some signal, trapping, and PTRACE_CONT with zero "data" should have the same effect.

Except that sending a signal will race with someone else sendng the same signal. It's nicer when this can be avoided.

Quoted 3 lines

Or do you mean that
instead of SIGSTOP, SIGTRAP should be sent? If so, yes, that would be much saner.

Yes, that's how it works in 3.1 already.

Quoted 10 lines

Quoted 2 lines

(3) "enable PTRACE_LISTEN. Works on group-stops even without any other options"

This is a symptom of the complexity I whined about above.

If ptrace has no influence on group stops then PTRACE_LISTEN is not needed at all. The problem seems to be that SIGSTOP state is the same as ptrace trapped state, or something like that. Group stops and ptrace stops should be totally orthogonal. Then crazy things like PTRACE_LISTEN are not needed.

Quoted 1 line

Why not just untangle the two concepts? (Via PTRACE_O_SANE.)

You are a bit late to the party. Thre was a LONG discussion how to implement proper group-stop handling. You need to write a much more detailed proposal than this vague suggestion "let's add one flag which makes everythng better". How EXACTLY this flag will change ptrace bhavior?

Quoted 5 lines

Quoted 2 lines

(5) "enable saner error codes"

Why do you need a new option for this? Ptrace error codes are close to useless, how can returning better error codes ever cause problems?

Now ESRCH means "no such process, or process is not attached to us, or process is not stopped (it is running), or process has exited (it is a zombie now)". I'd like to make it into four different error codes.

Quoted 8 lines

Quoted 3 lines

In fact, I am unsure why the new ops (points 2 and 3 above) need enabling. Maybe they can be simply be always allowed? Let's see...

My advise is to untangle all ptrace and group stop state from each other with one PTRACE_O_SANE option. That would replace options 1 to 4. This gives perhaps less choice, but considering how subtle it already is, I don't think that simplifying everything is a bad thing.

Again, please write a document how proposed PTRACE_O_SANE affects ptrace. *In details*.

Quoted 6 lines

Quoted 3 lines

Re PTRACE_INTERRUPT: "old-style" check for group-stop, namely, "if ptrace(PTRACE_GETSIGINFO) fails, then it's group-stop",

BTW, no one is going to use and know this, except perhaps ten people in the whole world.

It is documented behavior. Latest released strace already uses it.

Quoted 3 lines

Everyone else just thinks that ptrace doesn't pass on SIGSTOP, as the manpage says, and that the double SIGSTOP event is just a quirk.

We have an updated manpage. We are trying to push it into manpages git.

Quoted 3 lines

Quoted 1 line

if (!WIFSTOPPED(status)) ... /* error, should not happen */

It may happen if you're tracing a child process and are getting a normal SIGCHLD?

What?

Quoted 7 lines

Quoted 6 lines

...and here we decide to use either PTRACE_SYSCALL or PTRACE_LISTEN, _based on the value of "stopped" variable_. Here we PTRACE_LISTEN only if use_seize is true, but we use that flag only as indicator "this is a new kernel". It's easy to see that there are no intrisic reasons
why it can't also work on newer kernels, but after using old-style ATTACH.

Quoted 4 lines

So you're emulating normal group stop behaviour in the tracer, adding band-aids that make that possible, instead of avoiding these problems by making ptrace totally transparent for group stop state? Wouldn't it all be much simpler if ptrace didn't mess with group stop state?

As I said, gdb already depends on current ptrace behavior. We can't break it.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

(b) without any options, make SEIZE just do "ATTACH sans SIGSTOP" thing, nothing more.
(c) make the following new PTRACE_O_foo options:
(1) "flag stops with PTRACE_EVENT_STOP event value in waitpid status"

Why would anyone use that flag?

Well, it's nice to not require GETSIGINFO in order to know whether this stop is group-stop.

Yes, but that's theory. But in practice..? Try to take a step back and look at the big picture.

Most ptrace users aren't interested in group stops and just treat the two events the same (because they don't expect the SIGSTOP notification when WUNTRACED isn't set).

And group stop doesn't work on traced tasks

group stop isn't interesting (as it's
broken),

so why do a GETSIGINFO either? I totally agree that if you do
a GETSIGINFO now, this new option makes it easier,

but I'm trying to
say that almost no user will do the GETSIGINFO either.

The ptrace users who do want group stop to work usually don't want to interfere with it, they just want to know about it.

PTRACE_EVENT_STOP
makes the knowing slightly easier, but it doesn't fix group stop.

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

(2) "enable PTRACE_INTERRUPT. It either causes PTRACE_EVENT_STOP with sig=SIGTRAP if (1) is enabled, or creates a group-stop with sig=SIGTRAP otherwise" [if the second part is too weird to implement, make (2) require (1)]

Quoted 5 lines

Quoted 2 lines

I don't understand the need for PTRACE_INTERRUPT: Sending some signal, trapping, and PTRACE_CONT with zero "data" should have the same effect.

Except that sending a signal will race with someone else sendng the same signal. It's nicer when this can be avoided.

That race is not important, because the signal you send you will block (data=0), so the tracee will only see one signal.

Quoted 4 lines

Quoted 2 lines

Or do you mean that
instead of SIGSTOP, SIGTRAP should be sent? If so, yes, that would be much saner.

Yes, that's how it works in 3.1 already.

But don't existing programs depend on current ptrace behavior?

Quoted 9 lines

Quoted 9 lines

Quoted 1 line

(3) "enable PTRACE_LISTEN. Works on group-stops even without any other options"

This is a symptom of the complexity I whined about above.

If ptrace has no influence on group stops then PTRACE_LISTEN is not needed at all. The problem seems to be that SIGSTOP state is the same as ptrace trapped state, or something like that. Group stops and ptrace stops should be totally orthogonal. Then crazy things like PTRACE_LISTEN are not needed.

Quoted 6 lines

Quoted 1 line

Why not just untangle the two concepts? (Via PTRACE_O_SANE.)

You are a bit late to the party. Thre was a LONG discussion how to implement proper group-stop handling. You need to write a much more detailed proposal than this vague suggestion "let's add one flag which makes everythng better". How EXACTLY this flag will change ptrace bhavior?

I know! I'm fully aware of this. But I was too late for that long discussion, and after that I couldn't keep up with all patchsets sent out. It makes little sense to give detailed feedback when things might have totally changed again. To be convincing at all I'd probably need to write patches too, so show the advantages. But I never got to it. And at the time it was less clear to me what the real behaviour regarding group stop is, as you guys kindly explained to me.

----------------------------

To summarize, the flag would make ptrace have no effect on group stop state,

making SIGSTOP behaviour like any other signal behaves with ptrace. To get there, when PTRACE_O_SANE is set, the user space behaviour changes seen are as follows:

- In cases that ptrace sends a SIGSTOP, SIGTRAP is sent instead.

(Your 4th proposal). So that ptrace never changes the group stop state of a task.

- waitid() honours the absence of the WUNTRACED flag.

This prevents duplicate SIGSTOP events and the need to distinguish them from each other. No need for C.1 (PTRACE_EVENT_STOP). But if you still want it, it should be enabled by PTRACE_O_SANE too.

- PTRACE_CONT does not continue a stopped task.

Tracer does not need to keep track of group stop state anymore, SIGSTOP and SIGCONT just work for traced tasks.

No need for PTRACE_LISTEN. Tracer has
still full control over stopped state of tracee because it can block SIGSTOP and SIGCONT signals, and send them itself.

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

Re PTRACE_INTERRUPT: "old-style" check for group-stop, namely, "if ptrace(PTRACE_GETSIGINFO) fails, then it's group-stop",

BTW, no one is going to use and know this, except perhaps ten people in the whole world.

It is documented behavior. Latest released strace already uses it.

Where is it documented? Keep in mind that a wrong description of what's happening was in the manpage for years and years, when %99 of all current ptrace using code was written.

And it's in strace because one of you added it, I guess. But strace has no use for it, it already knows that a task is stopped when the tracee gets a SIGSTOP.

Or in other words, strace doesn't want to use it,

but it
has to because it gets multiple events for one SIGSTOP. And strace would much prefer it if a SIGCONT on the tracee would continue it again.

You can't do anything much with the extra info,

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

...and here we decide to use either PTRACE_SYSCALL or PTRACE_LISTEN, _based on the value of "stopped" variable_. Here we PTRACE_LISTEN only if use_seize is true, but we use that flag only as indicator "this is a new kernel". It's easy to see that there are no intrisic reasons
why it can't also work on newer kernels, but after using old-style ATTACH.

So you're emulating normal group stop behaviour in the tracer, adding band-aids that make that possible, instead of avoiding these problems by making ptrace totally transparent for group stop state? Wouldn't it all be much simpler if ptrace didn't mess with group stop state?

Quoted 1 line

As I said, gdb already depends on current ptrace behavior. We can't break it.

Then it won't use all those new flags either, and adding anything new makes no sense.

The behaviour you're defending is generally subtle and unexpected behaviour that most ptrace users don't expect.

Quoted 10 lines · Indan Zupancic

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

(b) without any options, make SEIZE just do "ATTACH sans SIGSTOP" thing, nothing more.
(c) make the following new PTRACE_O_foo options:
(1) "flag stops with PTRACE_EVENT_STOP event value in waitpid status"

Why would anyone use that flag?

Well, it's nice to not require GETSIGINFO in order to know whether this stop is group-stop.

Quoted 6 lines

Yes, but that's theory. But in practice..? Try to take a step back and look at the big picture.

Most ptrace users aren't interested in group stops and just treat the two events the same (because they don't expect the SIGSTOP notification when WUNTRACED isn't set).

Which users? strace? gdb? Something else?

Quoted 1 line

And group stop doesn't work on traced tasks

Group-stop works, as in "ptraced task does stop on SIGSTOP". The problem is that we couldn't make it wait for SIGCONT without making it run.

Quoted 3 lines

group stop isn't interesting (as it's
broken),

Why do you think so?

Quoted 3 lines

so why do a GETSIGINFO either? I totally agree that if you do
a GETSIGINFO now, this new option makes it easier,

It is not a new behavior.

Quoted 2 lines

but I'm trying to
say that almost no user will do the GETSIGINFO either.

strace does. I don't know about gdb.

Quoted 3 lines

The ptrace users who do want group stop to work usually don't want to interfere with it, they just want to know about it.

The point is, they can't "not interfere" with it. __WALL implicitly reports group-stops.

Quoted 3 lines

PTRACE_EVENT_STOP
makes the knowing slightly easier, but it doesn't fix group stop.

Correct. PTRACE_LISTEN fixes it.

Quoted 12 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

(2) "enable PTRACE_INTERRUPT. It either causes PTRACE_EVENT_STOP with

sig=SIGTRAP

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

if (1) is enabled, or creates a group-stop with sig=SIGTRAP otherwise"
[if the second part is too weird to implement, make (2) require (1)]

I don't understand the need for PTRACE_INTERRUPT: Sending some signal, trapping, and PTRACE_CONT with zero "data" should have the same effect.

Except that sending a signal will race with someone else sendng the same signal. It's nicer when this can be avoided.

Quoted 2 lines

That race is not important, because the signal you send you will block (data=0), so the tracee will only see one signal.

Signals don't queue. If you send SIGSTOP, and someone else sends SIGSTOP too at the same time, only one SIGSTOP will be delivered.
If you suppress it, you lose that second SIGSTOP.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Or do you mean that
instead of SIGSTOP, SIGTRAP should be sent? If so, yes, that would be much saner.

Yes, that's how it works in 3.1 already.

But don't existing programs depend on current ptrace behavior?

Existing programs don't use PTRACE_INTERRUPT.

Quoted 25 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

(3) "enable PTRACE_LISTEN. Works on group-stops even without any other

options"

Quoted 8 lines

Quoted 8 lines

This is a symptom of the complexity I whined about above.

If ptrace has no influence on group stops then PTRACE_LISTEN is not needed at all. The problem seems to be that SIGSTOP state is the same as ptrace trapped state, or something like that. Group stops and ptrace stops should be totally orthogonal. Then crazy things like PTRACE_LISTEN are not needed.

Quoted 6 lines

Quoted 1 line

Why not just untangle the two concepts? (Via PTRACE_O_SANE.)

You are a bit late to the party. Thre was a LONG discussion how to implement proper group-stop handling. You need to write a much more detailed proposal than this vague suggestion "let's add one flag which makes everythng better". How EXACTLY this flag will change ptrace bhavior?

I know! I'm fully aware of this. But I was too late for that long discussion, and after that I couldn't keep up with all patchsets sent out. It makes little sense to give detailed feedback when things might have totally changed again. To be convincing at all I'd probably need to write patches too, so show the advantages. But I never got to it. And at the time it was less clear to me what the real behaviour regarding group stop is, as you guys kindly explained to me.

Quoted 3 lines

----------------------------

To summarize, the flag would make ptrace have no effect on group stop state,

Wait a second. How and when would you set this new flag?

Quoted 5 lines

making SIGSTOP behaviour like any other signal behaves with ptrace. To get there, when PTRACE_O_SANE is set, the user space behaviour changes seen are as follows:

Quoted 3 lines

- In cases that ptrace sends a SIGSTOP, SIGTRAP is sent instead.

(Your 4th proposal). So that ptrace never changes the group stop state of a task.

Problem. Now we interfere with SIGSTRAPs. Yes, there are users who want to be able to see real SIGTRAPs they send to the program,
or ones generated by, say, int3 instruction on x86.

Quoted 3 lines

- waitid() honours the absence of the WUNTRACED flag.

(WUNTRACED is a horrible name. I prefer to use its alias WSTOPPED)

"honours the absence of the WUNTRACED flag" per-tracee?
Or do you want to set ptrace flag *on the tracer*? That would be new.

How will strace or gdb show that process has stopped, if it doesn't know it? With SIGSTOP it's not really important (user can infer that), but what about SIGTSTP? If strace says "SIGTSTP was delivered", is process stopped now, or is it looping in the SIGTSTP handler?

Quoted 5 lines

This prevents duplicate SIGSTOP events and the need to distinguish them from each other. No need for C.1 (PTRACE_EVENT_STOP). But if you still want it, it should be enabled by PTRACE_O_SANE too.

I'm confused now. Does PTRACE_O_SANE disable or enable group-stop notifications?

Quoted 2 lines

- PTRACE_CONT does not continue a stopped task.

You just said we will not get group-stops. How we can PTRACE_CONT from group-stop if we don't get group-stop?

In case you meant that "if we request group-stop notifications by using __WALL | WSTOPPED, and we get group-stop notification, and we do PTRACE_CONT, then task does not run (it sits in group-stop until SIGCONT or death)",

then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

Quoted 3 lines

Tracer does not need to keep track of group stop state anymore, SIGSTOP and SIGCONT just work for traced tasks.

But if it *wants to*?

Quoted 4 lines

No need for PTRACE_LISTEN. Tracer has
still full control over stopped state of tracee because it can block SIGSTOP and SIGCONT signals, and send them itself.

SIGCONT's side effect of waking up from group-stop can't be blocked. SIGCONT always wakes up all threads in thread group. Using SIGCONT to control tracee will race with SIGCONTs from other sources.

This makes SIGCONT a too coarse instrument for the job.

Quoted 9 lines

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

Re PTRACE_INTERRUPT: "old-style" check for group-stop, namely, "if ptrace(PTRACE_GETSIGINFO) fails, then it's group-stop",

BTW, no one is going to use and know this, except perhaps ten people in the whole world.

It is documented behavior. Latest released strace already uses it.

Quoted 7 lines

Where is it documented? Keep in mind that a wrong description of what's happening was in the manpage for years and years, when %99 of all current ptrace using code was written.

And it's in strace because one of you added it, I guess. But strace has no use for it, it already knows that a task is stopped when the tracee gets a SIGSTOP.

No, it does not know it for the other three stopping signals, since they can have non-default handlers.

Quoted 1 line

Or in other words, strace doesn't want to use it,

I think it should show group-stops. It's useful information.

Quoted 3 lines

but it
has to because it gets multiple events for one SIGSTOP. And strace would much prefer it if a SIGCONT on the tracee would continue it again.

Yes. But gdb prefers otherwise. That's why we need PTRACE_LISTEN for strace, and PTRACE_CONT for gdb.

Quoted 1 line

You can't do anything much with the extra info,

strace can print it, a-la:

--- stopped by SIGTTIN ---

Quoted 14 lines

Quoted 13 lines

Quoted 11 lines

Quoted 6 lines

...and here we decide to use either PTRACE_SYSCALL or PTRACE_LISTEN, _based on the value of "stopped" variable_. Here we PTRACE_LISTEN only if use_seize is true, but we use that flag only as indicator "this is a new kernel". It's easy to see that there are no intrisic reasons
why it can't also work on newer kernels, but after using old-style ATTACH.

So you're emulating normal group stop behaviour in the tracer, adding band-aids that make that possible, instead of avoiding these problems by making ptrace totally transparent for group stop state? Wouldn't it all be much simpler if ptrace didn't mess with group stop state?

As I said, gdb already depends on current ptrace behavior. We can't break it.

Quoted 2 lines

Then it won't use all those new flags either, and adding anything new makes no sense.

Yes... until gdb will want to give user a choice after SIGSTOP: continue to sit in group-stop until SIGCONT (wasn't possible until PTRACE_LISTEN), or continue executing (gdb's current behavior if user uses "continue" command). Therefore, gdb needs a way to do both.

Quoted 3 lines

The behaviour you're defending is generally subtle and unexpected behaviour that most ptrace users don't expect.

What behavior do I defend? That we get both signal-stops and group-stops? I don't "defend" it, it's just what we _already have_. We need to do minimal amount of changes.

In short, you propose to make it possible to switch off group-stop notification.

This makes it necessary to decide whether you want to continue or to stop on SIGSTOP *beforehand*, when you enter waitpid().

My points are:
(1) in strace, this would deprive us of a useful bit of information about the process.

(2) in gdb case, this may be too constraining for us: we want to be able to decide what to do on group-stop *after* group-stop has happened!

Quoted 5 lines

Quoted 2 lines

The ptrace users who do want group stop to work usually don't want to interfere with it, they just want to know about it.

The point is, they can't "not interfere" with it. __WALL implicitly reports group-stops.

That can be changed. That is not documented behaviour of __WALL, __WALL is supposed to tell for which tasks to give notifications, not what kind. At least that's the impression after reading the manpage.

If people want group-stop reports, they set WUNTRACED/WSTOPPED.

Quoted 4 lines

Quoted 2 lines

PTRACE_EVENT_STOP
makes the knowing slightly easier, but it doesn't fix group stop.

Correct. PTRACE_LISTEN fixes it.

In a very convoluted way. It fixes the symptomps, but it doesn't fix the problem, it works around it.

Quoted 6 lines

Quoted 6 lines

making SIGSTOP behaviour like any other signal behaves with ptrace. To get there, when PTRACE_O_SANE is set, the user space behaviour changes seen are as follows:

- In cases that ptrace sends a SIGSTOP, SIGTRAP is sent instead.

Quoted 5 lines

Quoted 1 line

(Your 4th proposal). So that ptrace never changes the group stop state of a task.

Problem. Now we interfere with SIGSTRAPs. Yes, there are users who want to be able to see real SIGTRAPs they send to the program,
or ones generated by, say, int3 instruction on x86.

But SIGTRAP is ours, ptrace already sends SIGTRAPS at execve.

Only change is
that it also sends one for new childs instead of SIGSTOP.

In any case, this is not a new problem.

But in the normal case non-ptrace traps aren't seen by tracees, if the tracer takes some care (if it doesn't, then current program don't get their SIGTRAP either).

Quoted 4 lines

How will strace or gdb show that process has stopped, if it doesn't know it? With SIGSTOP it's not really important (user can infer that), but what about SIGTSTP? If strace says "SIGTSTP was delivered", is process stopped now, or is it looping in the SIGTSTP handler?

Never heard of SIGTSTP, I don't know TTY black magic.

Tracers that want to get group stop notifications will set WSTOPPED and get that information that way. But as ptrace won't generate any SIGSTOPs, they don't have to use GETSIGINFO to know if it came from ptrace or not: It never does.

Quoted 6 lines

Quoted 3 lines

This prevents duplicate SIGSTOP events and the need to distinguish them from each other. No need for C.1 (PTRACE_EVENT_STOP). But if you still want it, it should be enabled by PTRACE_O_SANE too.

I'm confused now. Does PTRACE_O_SANE disable or enable group-stop notifications?

Neither, it makes waitid() honour the presence and absence of the WSTOPPED flag. It gives that choice back to the tracer.

Quoted 1 line

Quoted 4 lines

Quoted 1 line

- PTRACE_CONT does not continue a stopped task.

You just said we will not get group-stops. How we can PTRACE_CONT from group-stop if we don't get group-stop?

Why would you want to do that? I think I don't understand you.

Quoted 4 lines

In case you meant that "if we request group-stop notifications by using __WALL | WSTOPPED, and we get group-stop notification, and we do PTRACE_CONT, then task does not run (it sits in group-stop until SIGCONT or death)",

Exactly.

A group-stop notification is not really a ptrace event (maybe it is now), so PTRACE_CONT wouldn't be needed. It's just a group stop notification, not a freeze, report and wait event, like signals.

Quoted 4 lines

then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

Is that true? Isn't a SIGCONT sent to the TID only for that thread instead of the whole group? That's slightly inconvenient indeed. Perhaps this limitation can be fixed? Might be troublesome for the main thread.

But this is for a group stop initiated by gdb, I suppose?

In that case gdb
can just let the threads hang in the signal delivery path, and continue them one by one, like it does now.

Any group signal would work for this, doesn't
have to be SIGSTOP. E.g. a queueing rt signal that is blocked and never seen by the tracee.

Quoted 9 lines

Quoted 3 lines

No need for PTRACE_LISTEN. Tracer has
still full control over stopped state of tracee because it can block SIGSTOP and SIGCONT signals, and send them itself.

SIGCONT's side effect of waking up from group-stop can't be blocked. SIGCONT always wakes up all threads in thread group. Using SIGCONT to control tracee will race with SIGCONTs from other sources.

Quoted 1 line

This makes SIGCONT a too coarse instrument for the job.

No kidding. Perhaps the solution is to not use group stops for this in the first place.

But PTRACE_CONT results in one traced task running while the rest is still stopped, that could be called an often unwanted side effect too.

Quoted 6 lines

Quoted 3 lines

but it
has to because it gets multiple events for one SIGSTOP. And strace would much prefer it if a SIGCONT on the tracee would continue it again.

Yes. But gdb prefers otherwise. That's why we need PTRACE_LISTEN for strace, and PTRACE_CONT for gdb.

Or gdb can stop using group stops to achieve what it wants. Gdb is the strange use case, strace-like ptrace usage is the more common one. So make the more common case simpler instead of complicating everything for a corner gdb use case (however important it is).

Gdb can't want to have both transparent group stop behaviour, and also hijack group stops for thread running control. Letting it hang in ptrace is fine for gdb, that's what it does now. Let it keep doing it, it can even let it hang in SIGSTOP delivery path, it doesn't really matter. Only change is that tasks are in trapped state instead of actually stopped. That is an improvement, because then unrelated SIGCONT's can't interfere with gdb's debugging. But with PTRACE_CONT not continuing group stopped tasks it gives gdb the ability to transparently debug externally stopped and started tasks. It still can stop all tasks and continue them one by one as it does now.

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

As I said, gdb already depends on current ptrace behavior. We can't break it.

Then it won't use all those new flags either, and adding anything new makes no sense.

Quoted 4 lines

Yes... until gdb will want to give user a choice after SIGSTOP: continue to sit in group-stop until SIGCONT (wasn't possible until PTRACE_LISTEN), or continue executing (gdb's current behavior if user uses "continue" command). Therefore, gdb needs a way to do both.

But are users expected to send SIGSTOPs manually when debugging, instead of hitting ctlr+z or whatever?

Anyway, gdb can give this choice when it receives the SIGSTOP:
If users want it to go into group stop, gdb does PTRACE_CONT. If users want to selectively continue running, gdb continues select threads while blocking the SIGSTOP delivery.

Quoted 6 lines

Quoted 2 lines

The behaviour you're defending is generally subtle and unexpected behaviour that most ptrace users don't expect.

What behavior do I defend? That we get both signal-stops and group-stops? I don't "defend" it, it's just what we _already have_. We need to do minimal amount of changes.

The group stop interference behaviour. I don't think that adding PTRACE_INTERRUPT, PTRACE_LISTEN and all those new options is the minimal amount of change, especially not when including the user space changes needed to make use of this.

Quoted 2 lines

In short, you propose to make it possible to switch off group-stop notification.

That is just part of the proposal, the main this is to not let PTRACE_CONT continue a stopped task.

Switching off group-stop notifications makes it
only easier to use ptrace when not specifically interested in group stops.

Quoted 2 lines

(2) in gdb case, this may be too constraining for us: we want to be able to decide what to do on group-stop *after* group-stop has happened!

See above, that should be still possible. Or am I missing something?

Quoted 11 lines · Indan Zupancic

Quoted 5 lines

Quoted 2 lines

The ptrace users who do want group stop to work usually don't want to interfere with it, they just want to know about it.

The point is, they can't "not interfere" with it. __WALL implicitly reports group-stops.

That can be changed. That is not documented behaviour of __WALL, __WALL is supposed to tell for which tasks to give notifications, not what kind. At least that's the impression after reading the manpage.

Quoted 1 line

If people want group-stop reports, they set WUNTRACED/WSTOPPED.

Well, that is your interpretation. The fact is, existing programs either deliberately use __WALL in order to see (among other things) group-stops of tracees; or use __WALL and don't really care about group-stops they are getting - but they were debugged and tested on the kernels where group-stops were delivered on __WALL, thus they might break if that will stop happening.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

PTRACE_EVENT_STOP
makes the knowing slightly easier, but it doesn't fix group stop.

Correct. PTRACE_LISTEN fixes it.

In a very convoluted way. It fixes the symptomps, but it doesn't fix the problem, it works around it.

Well, as far as I am concerned, PTRACE_LISTEN allows me to achieve what I need.

If you don't like it just on the conceptual grounds, it may be a matter of taste.

Do you see an actual problem with it, as in "something doesn't work right"?

Quoted 5 lines

Quoted 3 lines

Problem. Now we interfere with SIGSTRAPs. Yes, there are users who want to be able to see real SIGTRAPs they send to the program,
or ones generated by, say, int3 instruction on x86.

But SIGTRAP is ours, ptrace already sends SIGTRAPS at execve.

...whcih causes problems, and therefore we have PTRACE_O_TRACEEXEC to suppress this idiotic post-execve SIGTRAP.

Quoted 2 lines

Only change is
that it also sends one for new childs instead of SIGSTOP.

Racing with user's SIGTRAP does not fix the problem, it merely moves it to a different signal. PTRACE_EVENT_STOP thingy fixes the problem once and for all.

Quoted 2 lines

In any case, this is not a new problem.

That the problem is old doesn't mean we can ignore it.

Quoted 3 lines

But in the normal case non-ptrace traps aren't seen by tracees, if the tracer takes some care (if it doesn't, then current program don't get their SIGTRAP either).

I don't understand this part.

Quoted 6 lines

Quoted 5 lines

How will strace or gdb show that process has stopped, if it doesn't know it? With SIGSTOP it's not really important (user can infer that), but what about SIGTSTP? If strace says "SIGTSTP was delivered", is process stopped now, or is it looping in the SIGTSTP handler?

Quoted 6 lines

Never heard of SIGTSTP, I don't know TTY black magic.

Tracers that want to get group stop notifications will set WSTOPPED and get that information that way. But as ptrace won't generate any SIGSTOPs, they don't have to use GETSIGINFO to know if it came from ptrace or not: It never does.

Drats. In your proposal, if I'd set WSTOPPED, I will get group-stops, right?

How in your proposed solution can I "restart and cancel group-stop" after group-stop? And how can I "restart and wait for a SIGCONT"?

Today both strace and gdb want to know about group-stops. If you think they should not, well, tough luck: their maintainers think otherwise.

Quoted 3 lines

A group-stop notification is not really a ptrace event (maybe it is now), so PTRACE_CONT wouldn't be needed. It's just a group stop notification, not a freeze, report and wait event, like signals.

Yes, this would be a reasonable API model. PTRACE_CONT cancels group-stop, and *doing nothing* leaves task in the group-stop. One problem with this:
many ptrace ops (such as GETREGS) are only allowed in ptrace stops. If you don't consider group-stop to be a ptrace-stop, then no such ops are allowed. In fact, even restart ops need stopped tracee, so PTRACE_CONT is illegal too. IOW: in this model, kernel doesn't know when we decided to go down *do nothing* route. From kernel POW, it is confused. Maybe we do want to PTRACE_CONT, but just didn't get around to it?

Quoted 5 lines

Quoted 4 lines

then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

Quoted 3 lines

Is that true? Isn't a SIGCONT sent to the TID only for that thread instead of the whole group? That's slightly inconvenient indeed. Perhaps this limitation can be fixed? Might be troublesome for the main thread.

This is how stopping/starting works. It's per-process, not per-thread. Regardless of the thread it is sent to, stop signals stop all threads, and SIGCONT restarts all of them.

Quoted 2 lines

But this is for a group stop initiated by gdb, I suppose?

It is true for any king of group-stop.

Quoted 3 lines

In that case gdb
can just let the threads hang in the signal delivery path, and continue them one by one, like it does now.

It can't, since it doesn't know about signal handlers.

I repeat: it is not about signals sent by gdb. It is about siganls sent by anyone.

Quoted 12 lines

Quoted 11 lines

Quoted 4 lines

No need for PTRACE_LISTEN. Tracer has
still full control over stopped state of tracee because it can block SIGSTOP and SIGCONT signals, and send them itself.

SIGCONT's side effect of waking up from group-stop can't be blocked. SIGCONT always wakes up all threads in thread group. Using SIGCONT to control tracee will race with SIGCONTs from other sources.

This makes SIGCONT a too coarse instrument for the job.

Quoted 5 lines

No kidding. Perhaps the solution is to not use group stops for this in the first place.

But PTRACE_CONT results in one traced task running while the rest is still stopped, that could be called an often unwanted side effect too.

Yes. But gdb people want that. I and Oleg tried to persuade them to stop wanting that. The end result is that they persuaded us that it's needed.

In short: "screw gdb needs, the elegance of my idea is more important that their real world needs". Sorry, but we tried that, and it leads nowhere.

We don't know. We should handle the widest possible set of scenarios:
^Z, manual signals, signals sent by kernel on bg I/O (such as SIGTTIN)...

Quoted 5 lines

Anyway, gdb can give this choice when it receives the SIGSTOP:
If users want it to go into group stop, gdb does PTRACE_CONT. If users want to selectively continue running, gdb continues select threads while blocking the SIGSTOP delivery.

Here we go again. If gdb would block signal delivery, HOW DOES IT KNOW THAT THIS SIGNAL IS GOING TO CAUSE GROUP-STOP? Think about receiving SIGTSTP. Will it cause group-stop?

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

The behaviour you're defending is generally subtle and unexpected behaviour that most ptrace users don't expect.

What behavior do I defend? That we get both signal-stops and group-stops? I don't "defend" it, it's just what we _already have_. We need to do minimal amount of changes.

Quoted 4 lines

The group stop interference behaviour. I don't think that adding PTRACE_INTERRUPT, PTRACE_LISTEN and all those new options is the minimal amount of change, especially not when including the user space changes needed to make use of this.

Current group-stop fix is EXACTLY one new ptrace command: PTRACE_LISTEN. How you can make it more minimal, I have no idea.

Quoted 7 lines

Quoted 4 lines

In short, you propose to make it possible to switch off group-stop notification.

That is just part of the proposal, the main this is to not let PTRACE_CONT continue a stopped task.

This will break gdb. I told this several times already. Not acceptable.

Quoted 3 lines

Switching off group-stop notifications makes it
only easier to use ptrace when not specifically interested in group stops.

Solution in the search of a problem.

Quoted 5 lines

Quoted 3 lines

(2) in gdb case, this may be too constraining for us: we want to be able to decide what to do on group-stop *after* group-stop has happened!

See above, that should be still possible. Or am I missing something?

Yes, you miss the fact that inferring group-stop on signal delivery is not reliable. The correct way to do it is to not *infer* it, but *observe* it when it really happens.

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Problem. Now we interfere with SIGSTRAPs. Yes, there are users who want to be able to see real SIGTRAPs they send to the program,
or ones generated by, say, int3 instruction on x86.

But SIGTRAP is ours, ptrace already sends SIGTRAPS at execve.

...whcih causes problems, and therefore we have PTRACE_O_TRACEEXEC to suppress this idiotic post-execve SIGTRAP.

The SIGTRAP is still there, there is just an extra bit to distinguish it from normal SIGTRAPs.

Sending another SIGTRAP with another bit set, just as with PTRACE_O_TRACEEXEC, solves this issue.

Quoted 3 lines

Quoted 2 lines

Only change is
that it also sends one for new childs instead of SIGSTOP.

Quoted 3 lines

Racing with user's SIGTRAP does not fix the problem, it merely moves it to a different signal. PTRACE_EVENT_STOP thingy fixes the problem once and for all.

It doesn't really matter if it's SIGSTOP with an extra flag, or SIGTRAP with an extra flag set. But SIGTRAP is already used this way for fork and exec,

so using it always instead of sometimes SIGSTOP would be more consistent. It's also clearer what to do with the SIGTRAP as tracer: Block it so the tracee doesn't see it. With SIGSTOP it's less clear what to do. In current trace it doesn't matter what you do, in practice, and that's also what the manpage says. So using SIGTRAP with an extra bit set is cleaner.

Quoted 6 lines

Quoted 4 lines

Tracers that want to get group stop notifications will set WSTOPPED and get that information that way. But as ptrace won't generate any SIGSTOPs, they don't have to use GETSIGINFO to know if it came from ptrace or not: It never does.

Drats. In your proposal, if I'd set WSTOPPED, I will get group-stops, right?

Group stop notifications, yes, that was the idea.

Quoted 2 lines

How in your proposed solution can I "restart and cancel group-stop" after group-stop? And how can I "restart and wait for a SIGCONT"?

Not sure what you mean with restart, just send another SIGSTOP?

Blocking a group stop happens by blocking the SIGSTOP delivery to the tracee.

Each tracee gets a SIGSTOP ptrace event, it's not like only one gets it and that controls the state of the whole group.

Same for SIGCONT. I'll have to
double check this, but I think this is how it works.

Quoted 4 lines

Quoted 3 lines

A group-stop notification is not really a ptrace event (maybe it is now), so PTRACE_CONT wouldn't be needed. It's just a group stop notification, not a freeze, report and wait event, like signals.

Quoted 8 lines

Yes, this would be a reasonable API model. PTRACE_CONT cancels group-stop, and *doing nothing* leaves task in the group-stop. One problem with this:
many ptrace ops (such as GETREGS) are only allowed in ptrace stops. If you don't consider group-stop to be a ptrace-stop, then no such ops are allowed. In fact, even restart ops need stopped tracee, so PTRACE_CONT is illegal too. IOW: in this model, kernel doesn't know when we decided to go down *do nothing* route. From kernel POW, it is confused. Maybe we do want to PTRACE_CONT, but just didn't get around to it?

It's probably best to not change the current model for group stop notifications (when requested via WSTOPPED), as that is confusing, even if the new behaviour is better (which it probably isn't).

You're right that if a tracee requests group stop notifications, it probably wants to be able to poke around too sometimes, and then a ptrace slightly entwined with group stops is preferred. But it's not elegant. :-/

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

Is that true? Isn't a SIGCONT sent to the TID only for that thread instead of the whole group? That's slightly inconvenient indeed. Perhaps this limitation can be fixed? Might be troublesome for the main thread.

Quoted 3 lines

This is how stopping/starting works. It's per-process, not per-thread. Regardless of the thread it is sent to, stop signals stop all threads, and SIGCONT restarts all of them.

Yes, but every thread gets a SIGCONT

and the tracee will get a ptrace
event for each thread. It can let the threads it doesn't want to continue hang in the ptrace handling, and continue only one with PTRACE_CONT.

Any reason why this can't work?

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

In short, you propose to make it possible to switch off group-stop notification.

That is just part of the proposal, the main this is to not let PTRACE_CONT continue a stopped task.

This will break gdb. I told this several times already. Not acceptable.

It probably can use SIGCONT to achieve what it wants.

Letting PTRACE_CONT
not resume stopped tasks would fix normal group stops without any changes.

Only programs that want to resume single threads while holding the rest need to change their code slightly to let the tasks hang in SIGCONT instead of group stop notification.

Quoted 4 lines

Quoted 2 lines

Switching off group-stop notifications makes it
only easier to use ptrace when not specifically interested in group stops.

Solution in the search of a problem.

You're probably right that it's not worth the trouble. I'm fine if tracers always get a group stop notification, but if so, there should be an option that makes PTRACE_CONT not continue group stopped tasks.

What I want to avoid is adding extra specific code in the tracer when all it wants is to be transparent to group stops.

One way is to not get the group stop notifications, so the tracer doesn't accidentally continues the stopped tasks. To me this seems the simplest solution, but it has the problem that it only works for tracers not interested in group stop. To also make it work with group stops, my idea was to let PTRACE_CONT not continue stopped tasks. Instead of trapping tasks at the beginning of group stop, they can trap the tasks at group stop exit time, which is much more natural anyway (because it is group continuation that gdb wants to prevent, not group stop).

With PTRACE_LISTEN the tracee has to handle group stops specially, then, instead of doing nothing for group stop notifications, it has to use PTRACE_LISTEN to emulate group stop and continuation for the tracee. No one is going to bother to add code for this when they're not interested in group stops, especially not if the code is portable at all.

It's just not worth the trouble. The behaviour is too strange and different with PTRACE_LISTEN.

Compare that to:

"PTRACE_O_FOO makes PTRACE_CONT/PTRACE_SYSCALL/etc. not automatically
continue a group stopped process, so that SIGSTOP and SIGCONT work for ptraced processes. Tracers that want to control thread continuation can do that at SIGCONT time."

Quoted 9 lines · Indan Zupancic

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Problem. Now we interfere with SIGSTRAPs. Yes, there are users who want to be able to see real SIGTRAPs they send to the program,
or ones generated by, say, int3 instruction on x86.

But SIGTRAP is ours, ptrace already sends SIGTRAPS at execve.

...whcih causes problems, and therefore we have PTRACE_O_TRACEEXEC to suppress this idiotic post-execve SIGTRAP.

Quoted 2 lines

The SIGTRAP is still there, there is just an extra bit to distinguish it from normal SIGTRAPs.

No. SIGTRAP is no longer 'there'. The PTRACE_EVENT_EXEC happens in a different
place: after syscall-entry, but before syscall-exit. Also, it can't be blocked by signal mask. Old-style post-execve SIGTRAP happens after syscall exit, and can be blocked by signal mask.

IOW: PTRACE_EVENT_EXEC is not a signal at all. It just happens to have WSTOPSIG(status) == SIGTRAP.

Quoted 3 lines

It doesn't really matter if it's SIGSTOP with an extra flag, or SIGTRAP with an extra flag set. But SIGTRAP is already used this way for fork and exec,

SIGTRAP is used for fork? I don't think so.

Quoted 22 lines

so using it always instead of sometimes SIGSTOP would be more consistent. It's also clearer what to do with the SIGTRAP as tracer: Block it so the tracee doesn't see it. With SIGSTOP it's less clear what to do. In current trace it doesn't matter what you do, in practice, and that's also what the manpage says. So using SIGTRAP with an extra bit set is cleaner.

Quoted 6 lines

Quoted 4 lines

Tracers that want to get group stop notifications will set WSTOPPED and get that information that way. But as ptrace won't generate any SIGSTOPs, they don't have to use GETSIGINFO to know if it came from ptrace or not: It never does.

Drats. In your proposal, if I'd set WSTOPPED, I will get group-stops, right?

Group stop notifications, yes, that was the idea.

Quoted 2 lines

How in your proposed solution can I "restart and cancel group-stop" after group-stop? And how can I "restart and wait for a SIGCONT"?

Not sure what you mean with restart, just send another SIGSTOP?

Quoted 1 line

Blocking a group stop happens by blocking the SIGSTOP delivery to the tracee.

Here we go again.

I will write it in bold.

YOU MUST NOT BLOCK DELIVERY OF STOPPING SIGNALS! IT'S ***WRONG***!

I already told you numerous times that SIGTSTP, SISTTIN and SIGTTOU can have non-default handlers. Do you understand what this means? I guess you don't, since you never replied with the explanation how you propose to with this problem.

I also told you that SIGSTOP/SIGCONT are *process-wide*. Meaning that if you stop on SIGSTOP delivery to one thread of a multithreaded process and will not restart it via PTRACE_CONT, then THIS PARTICULAL THREAD will stop running, yes, but all other threads WILL NOT STOP.

Quoted 2 lines

Each tracee gets a SIGSTOP ptrace event, it's not like only one gets it and that controls the state of the whole group.

It's exactly the opposite. SIGSTOP signal-delivery-stop happens on *one* thread, and if tracer does not suppress it, then all threads go into group-stop.

Quoted 2 lines

Same for SIGCONT. I'll have to
double check this, but I think this is how it works.

Please do double-check.

Quoted 7 lines

It's probably best to not change the current model for group stop notifications (when requested via WSTOPPED), as that is confusing, even if the new behaviour is better (which it probably isn't).

You're right that if a tracee requests group stop notifications, it probably wants to be able to poke around too sometimes, and then a ptrace slightly entwined with group stops is preferred. But it's not elegant. :-/

How kernel knows that tracer is done with poking around, and now it's ok to wake up on SIGCONT?

Quoted 1 line

Any reason why this can't work?

Gosh......... I already told you why it won't work.

SIGCONT is magic. It wakes all group-stopped threads, and this effect can't be blocked *even via ptrace*. This means that restarting only one thread via SIGCONT is impossible.

Quoted 2 lines

Letting PTRACE_CONT
not resume stopped tasks would fix normal group stops without any changes.

Exactly our line of thought before gdb people shot down our idea.

Quoted 7 lines

Only programs that want to resume single threads while holding the rest need to change their code slightly to let the tasks hang in SIGCONT instead of group stop notification.

Quoted 3 lines

Quoted 2 lines

Switching off group-stop notifications makes it
only easier to use ptrace when not specifically interested in group stops.

Quoted 5 lines

Quoted 1 line

Solution in the search of a problem.

You're probably right that it's not worth the trouble. I'm fine if tracers always get a group stop notification, but if so, there should be an option that makes PTRACE_CONT not continue group stopped tasks.

Yes. Exactly. That "option" is called PTRACE_LISTEN.

Quoted 3 lines

What I want to avoid is adding extra specific code in the tracer when all it wants is to be transparent to group stops.

You mean, you want to avoid this atrocious bloat? -

(quoted diff) +3 -1

-	ptrace(PTRACE_CONT, pid, 0, sig);
+	int is_stopped = .... determine whether it's a group_stop...
+	if (is_stopped) ptrace(PTRACE_LISTEN, pid, 0, 0);
+	else            ptrace(PTRACE_CONT, pid, 0, sig);

Somehow this doesn't look like big problem to me.

Quoted 10 lines

One way is to not get the group stop notifications, so the tracer doesn't accidentally continues the stopped tasks. To me this seems the simplest solution, but it has the problem that it only works for tracers not interested in group stop. To also make it work with group stops, my idea was to let PTRACE_CONT not continue stopped tasks. Instead of trapping tasks at the beginning of group stop, they can trap the tasks at group stop exit time, which is much more natural anyway (because it is group continuation that gdb wants to prevent, not group stop).

Quoted 5 lines

With PTRACE_LISTEN the tracee has to handle group stops specially, then, instead of doing nothing for group stop notifications, it has to use PTRACE_LISTEN to emulate group stop and continuation for the tracee. No one is going to bother to add code for this when they're not interested in group stops, especially not if the code is portable at all.

Portable code using ptrace? I pity those who need to maintain that...

Quoted 3 lines

It's just not worth the trouble. The behaviour is too strange and different with PTRACE_LISTEN.

I disagree. See code fragment above. It's simple.

Quoted 7 lines

Compare that to:

"PTRACE_O_FOO makes PTRACE_CONT/PTRACE_SYSCALL/etc. not automatically
continue a group stopped process, so that SIGSTOP and SIGCONT work for ptraced processes. Tracers that want to control thread continuation can do that at SIGCONT time."

This doesn't allow to make a decision whether to honor or ignore group-stop *after* it happened.

In case you meant that "if we request group-stop notifications by using __WALL | WSTOPPED, and we get group-stop notification, and we do PTRACE_CONT, then task does not run (it sits in group-stop until SIGCONT or death)", then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

SIGCONT's side effect of waking up from group-stop can't be blocked. SIGCONT always wakes up all threads in thread group. Using SIGCONT to control tracee will race with SIGCONTs from other sources.

This makes SIGCONT a too coarse instrument for the job.

Yes... until gdb will want to give user a choice after SIGSTOP: continue to sit in group-stop until SIGCONT (wasn't possible until PTRACE_LISTEN), or continue executing (gdb's current behavior if user uses "continue" command). Therefore, gdb needs a way to do both.

Quoted 7 lines · Denys Vlasenko

In case you meant that "if we request group-stop notifications by using __WALL | WSTOPPED, and we get group-stop notification, and we do PTRACE_CONT, then task does not run (it sits in group-stop until SIGCONT or death)", then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

[...]

Quoted 6 lines

SIGCONT's side effect of waking up from group-stop can't be blocked. SIGCONT always wakes up all threads in thread group. Using SIGCONT to control tracee will race with SIGCONTs from other sources.

This makes SIGCONT a too coarse instrument for the job.

[...]

Quoted 4 lines

Yes... until gdb will want to give user a choice after SIGSTOP: continue to sit in group-stop until SIGCONT (wasn't possible until PTRACE_LISTEN), or continue executing (gdb's current behavior if user uses "continue" command). Therefore, gdb needs a way to do both.

Having thought a bit more about this, I think this is less of a problem than it seems, because for a group stop we get a ptrace event for each task, and this should be true for SIGCONT as well. So gdb could also always let the group stop happen, and only when prompted to do so by a user, continue one thread by sending SIGCONT and letting all the other threads hang in trapped state.

Quoted 21 lines · Indan Zupancic

Quoted 7 lines · Denys Vlasenko

In case you meant that "if we request group-stop notifications by using __WALL | WSTOPPED, and we get group-stop notification, and we do PTRACE_CONT, then task does not run (it sits in group-stop until SIGCONT or death)", then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

[...]

Quoted 6 lines

SIGCONT's side effect of waking up from group-stop can't be blocked. SIGCONT always wakes up all threads in thread group. Using SIGCONT to control tracee will race with SIGCONTs from other sources.

This makes SIGCONT a too coarse instrument for the job.

[...]

Quoted 4 lines

Yes... until gdb will want to give user a choice after SIGSTOP: continue to sit in group-stop until SIGCONT (wasn't possible until PTRACE_LISTEN), or continue executing (gdb's current behavior if user uses "continue" command). Therefore, gdb needs a way to do both.

Quoted 6 lines

Having thought a bit more about this, I think this is less of a problem than it seems, because for a group stop we get a ptrace event for each task, and this should be true for SIGCONT as well. So gdb could also always let the group stop happen, and only when prompted to do so by a user, continue one thread by sending SIGCONT and letting all the other threads hang in trapped state.

Won't work. SIGCONT unpauses all threads in the thread group, and _then_ it is delivered to one of the threads.

You can block
or ignore it, yes, but it is too late: the unpausing already happened, and blocking/ignoring will only affect SIGCONT handler execution, if the program has one.

Quoted 27 lines

Quoted 26 lines

Quoted 7 lines

In case you meant that "if we request group-stop notifications by using __WALL | WSTOPPED, and we get group-stop notification, and we do PTRACE_CONT, then task does not run (it sits in group-stop until SIGCONT or death)", then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

[...]

Quoted 6 lines

SIGCONT's side effect of waking up from group-stop can't be blocked. SIGCONT always wakes up all threads in thread group. Using SIGCONT to control tracee will race with SIGCONTs from other sources.

This makes SIGCONT a too coarse instrument for the job.

[...]

Quoted 4 lines

Yes... until gdb will want to give user a choice after SIGSTOP: continue to sit in group-stop until SIGCONT (wasn't possible until PTRACE_LISTEN), or continue executing (gdb's current behavior if user uses "continue" command). Therefore, gdb needs a way to do both.

Having thought a bit more about this, I think this is less of a problem than it seems, because for a group stop we get a ptrace event for each task, and this should be true for SIGCONT as well. So gdb could also always let the group stop happen, and only when prompted to do so by a user, continue one thread by sending SIGCONT and letting all the other threads hang in trapped state.

Quoted 2 lines

Won't work. SIGCONT unpauses all threads in the thread group, and _then_ it is delivered to one of the threads.

No, it is delivered to _all_ threads.

With current ptrace you never see a SIGCONT

Quoted 4 lines

You can block
or ignore it, yes, but it is too late: the unpausing already happened, and blocking/ignoring will only affect SIGCONT handler execution, if the program has one.

Not doing PTRACE_CONT will keep the thread hanging in trapped state. All threads get a SIGCONT, not only one, so you can pause all threads this way.

Quoted 27 lines · Indan Zupancic

Quoted 27 lines

Quoted 26 lines

Quoted 7 lines

In case you meant that "if we request group-stop notifications by using __WALL | WSTOPPED, and we get group-stop notification, and we do PTRACE_CONT, then task does not run (it sits in group-stop until SIGCONT or death)", then we have a problem: gdb can't use this interface, it needs to be able to restart the thread (only one thread, not all of them, so sending SIGCONT is not ok!) from the group-stop. Yes, it's weird, but it's the real requirement from gdb users.

[...]

Quoted 6 lines

SIGCONT's side effect of waking up from group-stop can't be blocked. SIGCONT always wakes up all threads in thread group. Using SIGCONT to control tracee will race with SIGCONTs from other sources.

This makes SIGCONT a too coarse instrument for the job.

[...]

Quoted 4 lines

Yes... until gdb will want to give user a choice after SIGSTOP: continue to sit in group-stop until SIGCONT (wasn't possible until PTRACE_LISTEN), or continue executing (gdb's current behavior if user uses "continue" command). Therefore, gdb needs a way to do both.

Having thought a bit more about this, I think this is less of a problem than it seems, because for a group stop we get a ptrace event for each task, and this should be true for SIGCONT as well. So gdb could also always let the group stop happen, and only when prompted to do so by a user, continue one thread by sending SIGCONT and letting all the other threads hang in trapped state.

Quoted 4 lines

Quoted 2 lines

Won't work. SIGCONT unpauses all threads in the thread group, and _then_ it is delivered to one of the threads.

No, it is delivered to _all_ threads.

Wrong.

Quoted 1 line

With current ptrace you never see a SIGCONT

Wrong. Even rather old strace 4.5.9 does show it.

Quoted 3 lines

Not doing PTRACE_CONT will keep the thread hanging in trapped state. All threads get a SIGCONT, not only one, so you can pause all threads this way.

As I said, you are wrong about SIGCONT.

Hi guys,

I added code to use PTRACE_SEIZE in strace and in the process had tasted how API looks like from userspace POV.

It is usable, but API feels somewhat quirky.

Consider the following: one of reasons why we added PTRACE_SEIZE is that existing ptrace API has unnecessary complications (quirks) such as SIGSTOP on attach, SIGTRAP after execve.

But whoever designed strace did not deliberately designed these quirks in, he thought it was a good, reasonable design. Only after a second, third, tenth look it became obvious in retrospect that some things are not exactly right.

Thankfully, quirks in new PTRACE_SEIZE code mostly have the nature of "unnecessarily invented entities" as opposed to problems in trying to use API in real world tasks, but I still think they are annoying enough to be looked at.

We already have a mechanism how to modify ptrace behavior: ptrace options. But now we introduce a different mechnism to do the same: by using SEIZE instead of ATTACH, we magically change some aspects of ptrace. In effect, SEIZE sets some options. And these "SEIZE options" can't be set or cleared by SETOPTIONS. This is stupid. Why can't we just add more options instead of inventing new entities? Why we overloaded SEIZE with two functions: "attach to, but don't SIGSTOP the tracee" and "change behaviour of ptrace on this tracee"?

If the argument is that we want to set options immediately at attach, then I completely agree: yes, we do! Moreover, we want to set some _ordinary_ options too, such as PTRACE_O_TRACEEXEC, and we can't do that even now, in improved API! It needs more improving.

So my proposal is:
(a) make SEIZE take a parameter "immediately set these options on attach"
(b) without any options, make SEIZE just do "ATTACH sans SIGSTOP" thing, nothing more.
(c) make the following new PTRACE_O_foo options:
(1) "flag stops with PTRACE_EVENT_STOP event value in waitpid status"
(2) "enable PTRACE_INTERRUPT. It either causes PTRACE_EVENT_STOP with sig=SIGTRAP if (1) is enabled, or creates a group-stop with sig=SIGTRAP otherwise" [if the second part is too weird to implement, make (2) require (1)]
(3) "enable PTRACE_LISTEN. Works on group-stops even without any other options"
(4) "make auto-attached children stop a-la INTERRUPT, not with SIGSTOP"
(5) "enable saner error codes"

A crude patch is below. It rolls points 1-4 from above into a single new option, PTRACE_O_TRACESTOP.

Quoted 2 lines

A crude patch is below. It rolls points 1-4 from above into a single new option, PTRACE_O_TRACESTOP.

For easier review, split it into two parts. Part 1 follows. (It is still not even compile-tested).

Quoted 2 lines

A crude patch is below. It rolls points 1-4 from above into a single new option, PTRACE_O_TRACESTOP.

For easier review, split it into two parts. Part 1 follows. (It is still not even compile-tested).

Add new PTRACE_O_TRACESTOP option, make it control new ptrace behavior.

Introduce new ptrace option, PTRACE_O_TRACESTOP. This makes API
more symmetric: every PTRACE_EVENT_event has corresponding PTRACE_O_TRACEevent now, as it used to have before PTRACE_SEIZE was introduced.

PTRACE_SEIZE does not assume PTRACE_O_TRACESTOP, but with this patch it allows any PTRACE_O_opts to be set at attach time

From the correctness pov, the patch is mostly correct. but you forgot

Quoted 2 lines · Denys Vlasenko

PTRACE_SEIZE does not assume PTRACE_O_TRACESTOP, but with this patch it allows any PTRACE_O_opts to be set at attach time

Well. This assumes that the only difference with PTRACE_SEIZE is the new stop/interrupt behaviour. I am not sure this is "safe" to assume.

Tejun, what do you think?

Quoted 7 lines · Denys Vlasenko

Add new PTRACE_O_TRACESTOP option, make it control new ptrace behavior.

Introduce new ptrace option, PTRACE_O_TRACESTOP. This makes API
more symmetric: every PTRACE_EVENT_event has corresponding PTRACE_O_TRACEevent now, as it used to have before PTRACE_SEIZE was introduced.

Quoted 2 lines

PTRACE_SEIZE does not assume PTRACE_O_TRACESTOP, but with this patch it allows any PTRACE_O_opts to be set at attach time

Well. This assumes that the only difference with PTRACE_SEIZE is the new stop/interrupt behaviour. I am not sure this is "safe" to assume.

Tejun, what do you think?

kernel/ptrace.c static int ptrace_regset(struct task_struct *task, int req, unsigned int type, · line 654

Quoted 5 lines

int ptrace_request(struct task_struct *child, long request,

kernel/ptrace.c +1 -1

 		   unsigned long addr, unsigned long data)
 {
-	bool seized = child->ptrace & PT_SEIZED;
+	bool stop_events_enabled = child->ptrace & PT_TRACE_STOP;

May be ptrace_event_enabled(child, PTRACE_EVENT_STOP) looks better... The same about other PT_TRACE_STOP checks, although this is cosmetic.

And. Given that you can set/clear PT_TRACE_STOP in ptrace_setoptions(), you need the locking.

Just for example. do_signal_stop() calls ptrace_trap_notify() and hits WARN_ON_ONCE(!PT_TRACE_STOP) because it was cleared in between.

Quoted 5 lines · Oleg Nesterov

And. Given that you can set/clear PT_TRACE_STOP in ptrace_setoptions(), you need the locking.

Just for example. do_signal_stop() calls ptrace_trap_notify() and hits WARN_ON_ONCE(!PT_TRACE_STOP) because it was cleared in between.

PTRACE_SETOPTIONS can be used only on stopped tracees. Can do_signal_stop() run on a tracee while it is stopped?

Exchange PT_TRACESYSGOOD and PT_PTRACE_CAP bit positions, which makes PT_option bits contiguous and therefore makes code in ptrace_setoptions() much simpler.

kernel/ptrace.c +3 -3

@@ -219,19 +219,23 @@ static int ptrace_attach(struct task_struct *task, long request,
 
 	/*
 	 * SEIZE will enable new ptrace behaviors which will be implemented
-	 * gradually.  SEIZE_DEVEL is used to prevent applications
+	 * gradually.  SEIZE_DEVEL bit is used to prevent applications
 	 * expecting full SEIZE behaviors trapping on kernel commits which
 	 * are still in the process of implementing them.
 	 *
 	 * Only test programs for new ptrace behaviors being implemented
 	 * should set SEIZE_DEVEL.  If unset, SEIZE will fail with -EIO.
 	 *
-	 * Once SEIZE behaviors are completely implemented, this flag and
-	 * the following test will be removed.
+	 * Once SEIZE behaviors are completely implemented, this flag
+	 * will be removed.
 	 */
 	retval = -EIO;

kernel/ptrace.c +0 -0

 static int ptrace_setoptions(struct task_struct *child, unsigned long data)

{

kernel/ptrace.c +0 -11

-	child->ptrace &= ~PT_TRACE_MASK;
-
-	if (data & PTRACE_O_TRACESYSGOOD)
-		child->ptrace |= PT_TRACESYSGOOD;
-
-	if (data & PTRACE_O_TRACEFORK)
-		child->ptrace |= PT_TRACE_FORK;
-
-	if (data & PTRACE_O_TRACEVFORK)
-		child->ptrace |= PT_TRACE_VFORK;
-

include/linux/ptrace.h line 95

Quoted 3 lines

include/linux/ptrace.h +3 -0

+#define PT_OPT_FLAG_SHIFT	3
+/* must be directly before PT_TRACE_event bits */
+#define PT_TRACESYSGOOD	0x00000008

This probably means PT_TRACESYSGOOD should be also defined as PT_EVENT_FLAG(0)

include/linux/ptrace.h line 97

Quoted 5 lines

/* PT_TRACE_* event enable flags */

include/linux/ptrace.h +1 -3

-#define PT_EVENT_FLAG_SHIFT	4
-#define PT_EVENT_FLAG(event)	(1 << (PT_EVENT_FLAG_SHIFT + (event) - 1))
-
+#define PT_EVENT_FLAG(event)	(1 << (PT_OPT_FLAG_SHIFT + (event)))

And ptrace_setoptions() does

	child->ptrace |= (data << PT_OPT_FLAG_SHIFT);

Now we should verify that

	PTRACE_O_XXX == 1 << PTRACE_EVENT_XXX;

for every XXX... Looks correct. But perhaps it makes sense to do this explicitely and redefine PTRACE_O_* via PTRACE_EVENT_*.

Quoted 9 lines

kernel/ptrace.c +6 -2

-	if (seize && !(flags & PTRACE_SEIZE_DEVEL))
-		goto out;
+	if (seize) {
+		if ((flags & ~(long)PTRACE_O_MASK) != PTRACE_SEIZE_DEVEL)
+			goto out;
+		flags &= ~PTRACE_SEIZE_DEVEL;
+	} else
+		flags = 0;

Quoted 1 line

audit_ptrace(task);

This chunk looks completely off-topic, why it is needed in this patch?

Quoted 13 lines

static int ptrace_setoptions(struct task_struct *child, unsigned long data) {

kernel/ptrace.c +0 -11

-	child->ptrace &= ~PT_TRACE_MASK;
-
-	if (data & PTRACE_O_TRACESYSGOOD)
-		child->ptrace |= PT_TRACESYSGOOD;
-
-	if (data & PTRACE_O_TRACEFORK)
-		child->ptrace |= PT_TRACE_FORK;
-
-	if (data & PTRACE_O_TRACEVFORK)
-		child->ptrace |= PT_TRACE_VFORK;
-

kernel/ptrace.c int ptrace_writedata(struct task_struct *tsk, char __user *src, unsigned long ds · line 517

Quoted 10 lines

kernel/ptrace.c +2 -8

-	if (data & PTRACE_O_TRACECLONE)
-		child->ptrace |= PT_TRACE_CLONE;
-
-	if (data & PTRACE_O_TRACEEXEC)
-		child->ptrace |= PT_TRACE_EXEC;
-
-	if (data & PTRACE_O_TRACEVFORKDONE)
-		child->ptrace |= PT_TRACE_VFORK_DONE;
+	if (data & ~(long)PTRACE_O_MASK)
+		return -EINVAL;

Oh, yes, I always hated this logic. We change ->ptrace first, then return -EINVAL if data is wrong.

But. Denys, I think this needs a separate patch. And of course, of course this can break things. Say, a poor application passes the unsupported bit along with the valid bits, and doesn't check the result. This works before this patch.

Fix clearing of task->ptrace if PTRACE_SETOPTIONS fails.

If PTRACE_SETOPTIONS fails, options should not be affected. This patch makes it so.

While at it, simplify a few things: