Page MenuHomeFreeBSD

ptrace(2): add PT_GET_CHILDREN
ClosedPublic

Authored by kib on Fri, Jul 17, 8:14 PM.
Tags
None
Referenced Files
F164348567: D58315.id182156.diff
Fri, Jul 31, 1:46 AM
F164344247: D58315.id182302.diff
Fri, Jul 31, 12:37 AM
F164335686: D58315.id.diff
Thu, Jul 30, 10:25 PM
F164331135: D58315.id182238.diff
Thu, Jul 30, 9:17 PM
F164322685: D58315.id182241.diff
Thu, Jul 30, 7:05 PM
F164263206: D58315.diff
Thu, Jul 30, 5:15 AM
Unknown Object (File)
Tue, Jul 28, 8:31 AM
Unknown Object (File)
Sun, Jul 26, 12:57 PM

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Fri, Jul 17, 8:14 PM

Add PTCHLD_TRACED_BY_ME.
Add space for future ABI expansion.

Why not implement this with procctl()? That seems more natural to me.

lib/libsys/ptrace.2
1048
1050
1056
1063
1070
1071
1072
1076
1079
1094
1096

I don't understand this flag. Isn't it equivalent to PTCHLD_TRACED?

sys/kern/sys_process.c
1009

I think this is meant to be checking pp->p_pptr == td->td_proc.

1937

data is signed, you should check for negative values before the signed/unsigned comparison.

kib marked 13 inline comments as done.Sun, Jul 19, 6:34 PM

Why not implement this with procctl()? That seems more natural to me.

The call is used by the capsicumized debugger to get hold of its children after NOTE_FORK is reported by a procdesc note. I do not want to start enabling procctl(2) for capsicum.

Besides, the intent there is to report automatically attached debuggees, I report all children for completeness.

lib/libsys/ptrace.2
1070

I started a new sentence.

1096

Not necessarily, if the parent is the debugger, the process is not orphaned.

kib marked 2 inline comments as done.

Fix PTCHLD_TRACED_BY_ME.
Check uap->data < 0.
Use more correct type for rsrv[] to avoid compat32 conversions.
Man page editing.

markj added inline comments.
sys/kern/sys_process.c
1967

I suspect we should assert num1 == num here. Otherwise, we should set td->td_retval[0] = num1 after the two loops.

This revision is now accepted and ready to land.Mon, Jul 20, 1:10 PM
kib marked an inline comment as done.Mon, Jul 20, 2:20 PM
kib added inline comments.
sys/kern/sys_process.c
1967

It is worse. Processes could change the result of the p_cansee() between loops.

So I need to count everything when measuring the space for the children array.

kib marked an inline comment as done.

Allocate children big enough to hold information about all children processes, regardless of the p_cansee() return value.

This revision now requires review to proceed.Mon, Jul 20, 2:21 PM
markj added inline comments.
lib/libsys/ptrace.2
1096

Ok, I see. The use of "orphan" is confusing. The comment above the definition of p_orphans in struct proc explains the true meaning, but "orphan" sounds like a terminal state; I would expect "orphan process" to mean "process whose parent has exited". In reality the orphan state is temporary.

1099
sys/kern/sys_process.c
1935
This revision is now accepted and ready to land.Mon, Jul 20, 8:12 PM
kib marked 3 inline comments as done.Mon, Jul 20, 8:18 PM
This revision was automatically updated to reflect the committed changes.
sys/kern/sys_process.c
1238

I tried fuzzing the new command. If multiple threads in the same process call ptrace(PT_GET_CHILDREN, getpid()) at the same time, then this MPASS may fail.

sys/kern/sys_process.c
1238

I suspect the source line changed, am I right?

sys/kern/sys_process.c
1238

It looks right to me, but I should have made my comment self-contained, sorry.

The bug is with P2_PTRACEREQ handling: if multiple threads in the same process invoke ptrace(PT_GET_CHILDREN, getpid(), ...), then the assertion on line 1258 of sys_process.c can fail. P2_PTRACEREQ is supposed to serialize kern_ptrace(), but in this case it does not.