Why I want PTRACE_O_TRACESTOP option

Why I want PTRACE_O_TRACESTOP option

[Denys Vlasenko]

Seeing that rationale behind my proposal to turn PT_SEIZED bit
into an ordinary ptrace option is not well understood,
I need to explain it better.

A bit of history I gleaned by comparing old kernels.

In the time immemorial, in pre-2.4.0 days, someone added
PTRACE_SETOPTIONS, as an architecture-specific ptrace op,
with a single option bit, PTRACE_O_TRACESYSGOOD == 1 (bit 0).
In 2.4.0, only i386, mips64, sh supported it.

In other words, someone introduced PTRACE_SETOPTIONS as
a fix for one of ptrace problems (namely, for the problem
that syscall stop is not distinguishable from SIGTRAP).

In 2.4.x kernels and in 2.5 until 2.5.45 inclusive,
more architectures copied this code, but no new options
were introduced.

Then Daniel Jacobowitz in 2002 consolidated cut-n-paste
code - made archtecture-independent PTRACE_SETOPTIONS
(that's why there is PTRACE_OLDSETOPTIONS - arch-dependent
ops have different range assigned to them), and added new
option bits: PTRACE_O_TRACEFORK, PTRACE_O_TRACEVFORK,
PTRACE_O_TRACECLONE, PTRACE_O_TRACEEXEC. (He did more
than this, but I track history of ptrace options here).
https://lkml.org/lkml/2002/9/18/291
https://lkml.org/lkml/2002/9/18/293
https://lkml.org/lkml/2002/9/18/294
These option bits took bit positions 1,2,3,4.
Corresponding generated PTRACE_EVENTs have values 1,2,3,4.
These options made it easier to catch children
of ptraced processes, and to suppress notorious
post-execve SIGTRAP.
These changes went into 2.5.46.

In 2003, Daniel added two more options - see
https://lkml.org/lkml/2003/2/6/160 -
PTRACE_O_TRACEVFORKDONE and PTRACE_O_TRACEEXIT.
Naturally, they took bits 5 and 6,
and corresponding PTRACE_EVENTs have values 5 and 6.
These options made it possible to examine process state
at exit, and to know when it's safe to reinsert breakpoints
into vfork parent.
These changes went into 2.5.60.

Now we came and decided to tackle next batch of ptrace bugs:
non-working group-stops and SIGSTOP races on (auto-)attach.
Second bug can't be fixed by PTRACE_SETOPTIONS, so we had to add
new attach command, PTRACE_SEIZE. We also added a new event,
PTRACE_EVENT_STOP, which took on next available value, 7.
Feeling emboldened, we hooked new group-stop machinery to the same
internal PT_SEIZE bit which indicates that tracee was seized,
instead of adding an option, as people before us did.

Consider what will happen when a next ptrace fix will require
a way to change ptrace API at runtime. A new option will likely
be introduced, say, PTRACE_O_TRACEPONY, with next available
bit position 7, and perhaps some new event will be generated,
PTRACE_EVENT_PONY, with value.... yes, it can't be 7,
PTRACE_EVENT_STOP took it. So it will probably be 8.

Which will look illogical. Why bit position doesn't match event value?
Why PTRACE_SEIZE people decided to be special and broke
established pattern of matching option bits and event values?

Of course, by that time it will be too late to fix it!


That's why I propose to not go down that path. Instead, try to match
the previous pattern of ptrace evolution more closely. Do not introduce
hidden PT_SEIZE bit. Instead, introduce just another option bit with bit
position 7. Since setting options on attach is needed anyway
and we are going to implement it in PTRACE_SEIZE, the "SIGSTOP races on
(auto-)attach can't be fixed by PTRACE_SETOPTIONS" problem no longer
exists: we _can_ set this new option on attach now.

That's how I ended up with the proposal to introduce option
PTRACE_O_TRACESTOP = (1 << 7) which matches PTRACE_EVENT_STOP = 7 we
already added.

I do not do it specifically in order to make fixed group-stop behavior
settable by PTRACE_SETOPTIONS too (even though this will be a side
effect of proposed change). In fact, in light of possible races on API
switch I think we should recommend to userspace to set all options
at attach time, via PTRACE_SEIZE. I believe even today there are races
already: setting PTRACE_O_TRACEEXEC can race with post-execve SIGTRAP
generation, for example. We won't be introducing a new problem.


Is my proposal clearer now? What do you guys think?

-- 
vda

Re: Why I want PTRACE_O_TRACESTOP option

[Tejun Heo]

Hello, Denys.

On Thu, Sep 08, 2011 at 06:50:01PM +0200, Denys Vlasenko wrote:
> Consider what will happen when a next ptrace fix will require
> a way to change ptrace API at runtime. A new option will likely
> be introduced, say, PTRACE_O_TRACEPONY, with next available
> bit position 7, and perhaps some new event will be generated,
> PTRACE_EVENT_PONY, with value.... yes, it can't be 7,
> PTRACE_EVENT_STOP took it. So it will probably be 8.

Then, just give it the next matching number.

If options naturally happen to match the events, that's a nice
coincidence.  If the real life requirement deviates from the beautiful
one-to-one mapping, then, so be it.  No, the magical contiguous one to
one mapping isn't the most important design concern.

To me, the rationale presented here almost argues against
PTRACE_O_TRACESTOP.  :(

Thanks.

-- 
tejun

Re: Why I want PTRACE_O_TRACESTOP option

[Denys Vlasenko]

On Friday 09 September 2011 02:18, Tejun Heo wrote:
> Hello, Denys.
> 
> On Thu, Sep 08, 2011 at 06:50:01PM +0200, Denys Vlasenko wrote:
> > Consider what will happen when a next ptrace fix will require
> > a way to change ptrace API at runtime. A new option will likely
> > be introduced, say, PTRACE_O_TRACEPONY, with next available
> > bit position 7, and perhaps some new event will be generated,
> > PTRACE_EVENT_PONY, with value.... yes, it can't be 7,
> > PTRACE_EVENT_STOP took it. So it will probably be 8.
> 
> Then, just give it the next matching number.
> 
> If options naturally happen to match the events, that's a nice
> coincidence.  If the real life requirement deviates from the beautiful
> one-to-one mapping, then, so be it.  No, the magical contiguous one to
> one mapping isn't the most important design concern.
> 
> To me, the rationale presented here almost argues against
> PTRACE_O_TRACESTOP.  :(

How about changing PTRACE_EVENT_STOP value to, say, 255 then,
in order to move it away from (option,event) area?

-- 
vda

Re: Why I want PTRACE_O_TRACESTOP option

[Denys Vlasenko]

On Friday 09 September 2011 02:18, Tejun Heo wrote:
> Hello, Denys.
> 
> On Thu, Sep 08, 2011 at 06:50:01PM +0200, Denys Vlasenko wrote:
> > Consider what will happen when a next ptrace fix will require
> > a way to change ptrace API at runtime. A new option will likely
> > be introduced, say, PTRACE_O_TRACEPONY, with next available
> > bit position 7, and perhaps some new event will be generated,
> > PTRACE_EVENT_PONY, with value.... yes, it can't be 7,
> > PTRACE_EVENT_STOP took it. So it will probably be 8.
> 
> Then, just give it the next matching number.

My point is that previously, ptrace behavior was modified by setting
options. Why don't we use this mechanism? Why we invent a different
wheel? Ptrace is ugly as-is, why complicate it even further?

The argument was that SETOPTIONS wasn't suitable for modifying
attach behavior, but this is fixed by "set options on SEIZE"
patch. I don't see why we can't use options mechanist to affect
group-stop behavior now.

-- 
vda

Re: Why I want PTRACE_O_TRACESTOP option

[Indan Zupancic]

Hello,

On Fri, September 9, 2011 07:54, Denys Vlasenko wrote:
> On Friday 09 September 2011 02:18, Tejun Heo wrote:
>> Hello, Denys.
>>
>> On Thu, Sep 08, 2011 at 06:50:01PM +0200, Denys Vlasenko wrote:
>> > Consider what will happen when a next ptrace fix will require
>> > a way to change ptrace API at runtime. A new option will likely
>> > be introduced, say, PTRACE_O_TRACEPONY, with next available
>> > bit position 7, and perhaps some new event will be generated,
>> > PTRACE_EVENT_PONY, with value.... yes, it can't be 7,
>> > PTRACE_EVENT_STOP took it. So it will probably be 8.
>>
>> Then, just give it the next matching number.
>
> My point is that previously, ptrace behavior was modified by setting
> options. Why don't we use this mechanism? Why we invent a different
> wheel? Ptrace is ugly as-is, why complicate it even further?
>
> The argument was that SETOPTIONS wasn't suitable for modifying
> attach behavior, but this is fixed by "set options on SEIZE"
> patch. I don't see why we can't use options mechanist to affect
> group-stop behavior now.

I totally agree with Denys here.

It is very useful to set options atomically at SEIZE time. Having
SEIZE set some hidden option implicitly only makes things more
confusing and harder to explain what SEIZE does. Please apply Denys'
SEIZE API improvements.

Another important reason to make PTRACE_O_TRACESTOP an option is
because not everyone uses SEIZE: Users using PTRACE_TRACEME can't
set this option at all. For those PTRACE_O_TRACESTOP is needed.
Using PTRACE_TRACEME is the common case for both strace and gdb.

That is an indication that mixing up the setting of an option with
a specific attach request is wrong.

Greetings,

Indan


P.S. It seems messages to tj@kernel.org have trouble getting through,
I hope they arrive eventually.

Re: Why I want PTRACE_O_TRACESTOP option

[Indan Zupancic]

On Fri, September 9, 2011 14:26, Indan Zupancic wrote:
> Hello,
>
> On Fri, September 9, 2011 07:54, Denys Vlasenko wrote:
>> On Friday 09 September 2011 02:18, Tejun Heo wrote:
>>> Hello, Denys.
>>>
>>> On Thu, Sep 08, 2011 at 06:50:01PM +0200, Denys Vlasenko wrote:
>>> > Consider what will happen when a next ptrace fix will require
>>> > a way to change ptrace API at runtime. A new option will likely
>>> > be introduced, say, PTRACE_O_TRACEPONY, with next available
>>> > bit position 7, and perhaps some new event will be generated,
>>> > PTRACE_EVENT_PONY, with value.... yes, it can't be 7,
>>> > PTRACE_EVENT_STOP took it. So it will probably be 8.
>>>
>>> Then, just give it the next matching number.
>>
>> My point is that previously, ptrace behavior was modified by setting
>> options. Why don't we use this mechanism? Why we invent a different
>> wheel? Ptrace is ugly as-is, why complicate it even further?
>>
>> The argument was that SETOPTIONS wasn't suitable for modifying
>> attach behavior, but this is fixed by "set options on SEIZE"
>> patch. I don't see why we can't use options mechanist to affect
>> group-stop behavior now.
>
> I totally agree with Denys here.
>
> It is very useful to set options atomically at SEIZE time. Having
> SEIZE set some hidden option implicitly only makes things more
> confusing and harder to explain what SEIZE does. Please apply Denys'
> SEIZE API improvements.
>
> Another important reason to make PTRACE_O_TRACESTOP an option is
> because not everyone uses SEIZE: Users using PTRACE_TRACEME can't
> set this option at all. For those PTRACE_O_TRACESTOP is needed.
> Using PTRACE_TRACEME is the common case for both strace and gdb.

PTRACE_O_TRACESTOP only makes sense if it also affects auto-attach
SIGSTOPS, of course. I don't know if it does. It probably should.

Indan

Re: Why I want PTRACE_O_TRACESTOP option

[Oleg Nesterov]

On 09/09, Denys Vlasenko wrote:
>
> How about changing PTRACE_EVENT_STOP value to, say, 255 then,
> in order to move it away from (option,event) area?

Probably makes sense.

Oleg.

Re: Why I want PTRACE_O_TRACESTOP option

[Oleg Nesterov]

On 09/09, Tejun Heo wrote:
>
> To me, the rationale presented here almost argues against
> PTRACE_O_TRACESTOP.  :(

Personally I dislike PTRACE_O_TRACESTOP too.

And once again, this needs more (nontrivial) changes than just
s/PT_SEIZED/PTRACE_O_TRACESTOP/, the patch was wrong.

And, just for example. While this is minor, I'd like to improve
the error codes from ptrace_check_attach(). Until we drop
PTRACE_SEIZE_DEVEL we can safely do this. I don't think it makes
sense to add the new PTRACE_O for this.

And finally, I simply do not understand how it can be useful.

Oleg.

Re: Why I want PTRACE_O_TRACESTOP option

[Oleg Nesterov]

On 09/09, Indan Zupancic wrote:
>
> It is very useful to set options atomically at SEIZE time.

Nobody argues with this.

> Another important reason to make PTRACE_O_TRACESTOP an option is
> because not everyone uses SEIZE: Users using PTRACE_TRACEME can't
> set this option at all.

Yes. This was already discussed, PTRACE_TRACEME obviously doesn't
work if you need the new features. So far it was decided TRACEME
should be avoided, but perhaps we can add SEIZE_ME. And, unlike
TRACEME it should probably stop immediately to simplify the
synchronization with parent. Afaik, any user of TRACEME does
something like kill(getpid(), SIGSTOP), this doesn't look very
good.

But personally I'd prefer to avoid SEIZE_ME.

Oleg.

Re: Why I want PTRACE_O_TRACESTOP option

[Denys Vlasenko]

On Fri, 2011-09-09 at 15:01 +0200, Indan Zupancic wrote:
> On Fri, September 9, 2011 14:26, Indan Zupancic wrote:
> > Hello,
> >
> > On Fri, September 9, 2011 07:54, Denys Vlasenko wrote:
> >> On Friday 09 September 2011 02:18, Tejun Heo wrote:
> >>> Hello, Denys.
> >>>
> >>> On Thu, Sep 08, 2011 at 06:50:01PM +0200, Denys Vlasenko wrote:
> >>> > Consider what will happen when a next ptrace fix will require
> >>> > a way to change ptrace API at runtime. A new option will likely
> >>> > be introduced, say, PTRACE_O_TRACEPONY, with next available
> >>> > bit position 7, and perhaps some new event will be generated,
> >>> > PTRACE_EVENT_PONY, with value.... yes, it can't be 7,
> >>> > PTRACE_EVENT_STOP took it. So it will probably be 8.
> >>>
> >>> Then, just give it the next matching number.
> >>
> >> My point is that previously, ptrace behavior was modified by setting
> >> options. Why don't we use this mechanism? Why we invent a different
> >> wheel? Ptrace is ugly as-is, why complicate it even further?
> >>
> >> The argument was that SETOPTIONS wasn't suitable for modifying
> >> attach behavior, but this is fixed by "set options on SEIZE"
> >> patch. I don't see why we can't use options mechanist to affect
> >> group-stop behavior now.
> >
> > I totally agree with Denys here.
> >
> > It is very useful to set options atomically at SEIZE time. Having
> > SEIZE set some hidden option implicitly only makes things more
> > confusing and harder to explain what SEIZE does. Please apply Denys'
> > SEIZE API improvements.
> >
> > Another important reason to make PTRACE_O_TRACESTOP an option is
> > because not everyone uses SEIZE: Users using PTRACE_TRACEME can't
> > set this option at all. For those PTRACE_O_TRACESTOP is needed.
> > Using PTRACE_TRACEME is the common case for both strace and gdb.
> 
> PTRACE_O_TRACESTOP only makes sense if it also affects auto-attach
> SIGSTOPS, of course. I don't know if it does. It probably should.

With the patch, it will affect them if it is used in list of options
passed on SEIZE.

-- 
vda

Re: Why I want PTRACE_O_TRACESTOP option

[Indan Zupancic]

On Fri, September 9, 2011 18:26, Oleg Nesterov wrote:
> On 09/09, Indan Zupancic wrote:
>>
>> It is very useful to set options atomically at SEIZE time.
>
> Nobody argues with this.
>
>> Another important reason to make PTRACE_O_TRACESTOP an option is
>> because not everyone uses SEIZE: Users using PTRACE_TRACEME can't
>> set this option at all.
>
> Yes. This was already discussed, PTRACE_TRACEME obviously doesn't
> work if you need the new features. So far it was decided TRACEME
> should be avoided,

How do you want to attach/seize a just forked child without races
in a less ugly way than with TRACEME?

> but perhaps we can add SEIZE_ME. And, unlike
> TRACEME it should probably stop immediately to simplify the
> synchronization with parent. Afaik, any user of TRACEME does
> something like kill(getpid(), SIGSTOP), this doesn't look very
> good.

I let the child send a SIGTERM to itself, so if anything goes wrong
it terminates. It's only needed because you have to set options.

>
> But personally I'd prefer to avoid SEIZE_ME.

Me too. There is no need for it if PTRACE_O_TRACESTOP exists.

Alternatively, if you do want to add it, just allow PTRACE_SEIZE
with zero/own PID to achieve the same.

Greetings,

Indan

Re: Why I want PTRACE_O_TRACESTOP option

[Denys Vlasenko]

On Saturday 10 September 2011 01:09, Indan Zupancic wrote:
> On Fri, September 9, 2011 18:26, Oleg Nesterov wrote:
> > On 09/09, Indan Zupancic wrote:
> >>
> >> It is very useful to set options atomically at SEIZE time.
> >
> > Nobody argues with this.
> >
> >> Another important reason to make PTRACE_O_TRACESTOP an option is
> >> because not everyone uses SEIZE: Users using PTRACE_TRACEME can't
> >> set this option at all.
> >
> > Yes. This was already discussed, PTRACE_TRACEME obviously doesn't
> > work if you need the new features. So far it was decided TRACEME
> > should be avoided,
> 
> How do you want to attach/seize a just forked child without races
> in a less ugly way than with TRACEME?

I needed to do it when I was adding usage of SEIZE to strace.
It goes like this:

- fork
- child: raise(SIGSTOP)
- parent: waits until it sees child stopping
- parent: seizes the child
- parent: kill(child, SIGCONT)


-- 
vda

Re: Why I want PTRACE_O_TRACESTOP option

[Pedro Alves]

On Saturday 10 September 2011 02:17:03, Denys Vlasenko wrote:
> On Saturday 10 September 2011 01:09, Indan Zupancic wrote:
> > On Fri, September 9, 2011 18:26, Oleg Nesterov wrote:
> > > On 09/09, Indan Zupancic wrote:
> > >>
> > >> It is very useful to set options atomically at SEIZE time.
> > >
> > > Nobody argues with this.
> > >
> > >> Another important reason to make PTRACE_O_TRACESTOP an option is
> > >> because not everyone uses SEIZE: Users using PTRACE_TRACEME can't
> > >> set this option at all.
> > >
> > > Yes. This was already discussed, PTRACE_TRACEME obviously doesn't
> > > work if you need the new features. So far it was decided TRACEME
> > > should be avoided,
> > 
> > How do you want to attach/seize a just forked child without races
> > in a less ugly way than with TRACEME?
> 
> I needed to do it when I was adding usage of SEIZE to strace.
> It goes like this:
> 
> - fork
> - child: raise(SIGSTOP)
> - parent: waits until it sees child stopping
> - parent: seizes the child
> - parent: kill(child, SIGCONT)

You can do without SIGSTOP/signals:

- pipe
- fork
- child: block reading pipe
- parent: seizes the child
- parent: whatever else needs doing (e.g, interrupt and set options if not already done at seize time)
- parent: close the pipe
- child: notices pipe closed, moves on.

GDB already does something similar for other OSs (darwin/OSX, ttrace/HP-UX).

-- 
Pedro Alves

Re: Why I want PTRACE_O_TRACESTOP option

[Tejun Heo]

On Fri, Sep 09, 2011 at 06:00:48PM +0200, Oleg Nesterov wrote:
> On 09/09, Denys Vlasenko wrote:
> >
> > How about changing PTRACE_EVENT_STOP value to, say, 255 then,
> > in order to move it away from (option,event) area?
> 
> Probably makes sense.

Yeap, no objection from me.

Thanks.

-- 
tejun

Re: Why I want PTRACE_O_TRACESTOP option

[Tejun Heo]

Hello, Denys.

On Fri, Sep 09, 2011 at 07:54:50AM +0200, Denys Vlasenko wrote:
> My point is that previously, ptrace behavior was modified by setting
> options. Why don't we use this mechanism? Why we invent a different
> wheel? Ptrace is ugly as-is, why complicate it even further?
> 
> The argument was that SETOPTIONS wasn't suitable for modifying
> attach behavior, but this is fixed by "set options on SEIZE"
> patch. I don't see why we can't use options mechanist to affect
> group-stop behavior now.

The argument was that there are other more difficult issues and the
added benefit - superficial consistency - doesn't justify the
necessary complexity, and it was repeated multiple times.

Thanks.

-- 
tejun