ptrace(2): add PT_SC_REMOTE remote syscall request

[markj]

What's the motivation for adding this feature?

sys/kern/kern_sig.c

2682

Should we try to respect trap_enotcap somehow?

2685

Why not try to update td_cowgen here?

2725

The duplication between this function and syscallenter() isn't ideal. I wonder if the latter can be broken up into multiple inline functions, and some of them reused here.

2766

Extra newline.

[kib]

sys/kern/kern_sig.c

2682

I thought about it, and decided not to do. The main reason is that I do not additionally add a recursion to generate new signal from ptracestop() (it is there anyway, but I do not want to add more). Logical reason is that the code is requested by the debugger already, so it should be capable to see the ECAPMODE by itself, without additional help.

If you still consider it useful to generate SIGTRAP there, please explain.

2725

I considered it very briefly, but the functions appear to be too small and ad-hoc, IMO. This can be reconsidered, but for me it does not look compelling.

[kib]

In D37590#854955, @markj wrote:

What's the motivation for adding this feature?

There are some programs doing exactly that, by hacking a place in the .text and injecting their code. I remember that reptyr (ports/sysutils/reptyr) does that. Also some time ago I listen to a talk describing the same hack. Since we already has the hardest part done, the ability to request a code to be executed in the remote context (for coredumps), I finally decided to add this feature in a regular and hopefully more convenient way.

This is the primary reason why Kyle is added as a reviewer.

[emaste]

lib/libc/sys/ptrace.2

1015

Could we add some context about which thread is chosen?

[kib]

Update CoW fields for curthread on remote syscall req.

[kib]

ptrace(2): add a section on the target selection

[jhb]

Hmm, GDB implements this sort of thing by changing registers to point to a block of code inserted into the address space.

[kib]

In D37590#855703, @jhb wrote:

Hmm, GDB implements this sort of thing by changing registers to point to a block of code inserted into the address space.

Exactly. This is what I am trying to provide a (much) more convenient interface for.

[kevans]

+1; iirc this kills off the only MD part of reptyr, which was mainly the registers that it swaps out for syscall args. It doesn't go as far as a whole block of code, though, iirc- just proceed to syscall entry, swap out syscall args with its own, then rewind %Ip to execute victim's syscall. Still uglier than it needs to be...

[jhb]

In D37590#855715, @kib wrote:

In D37590#855703, @jhb wrote:

Hmm, GDB implements this sort of thing by changing registers to point to a block of code inserted into the address space.

Exactly. This is what I am trying to provide a (much) more convenient interface for.

Well, except that GDB does this for all sorts of things that is more general than just invoking a system call. I can't see it ever making use of this as it is too limited. That said, there's lots of this that GDB does more of on Linux than on FreeBSD, and it might be that using this to call mmap(MAP_ANON...) to get the initial set of scratch space to which compiled code can be written might be useful. Also for things like displaced stepping that is not implemented on FreeBSD.

[jhb]

In D37590#855732, @jhb wrote:

In D37590#855715, @kib wrote:

In D37590#855703, @jhb wrote:

Hmm, GDB implements this sort of thing by changing registers to point to a block of code inserted into the address space.

Exactly. This is what I am trying to provide a (much) more convenient interface for.

Well, except that GDB does this for all sorts of things that is more general than just invoking a system call. I can't see it ever making use of this as it is too limited. That said, there's lots of this that GDB does more of on Linux than on FreeBSD, and it might be that using this to call mmap(MAP_ANON...) to get the initial set of scratch space to which compiled code can be written might be useful. Also for things like displaced stepping that is not implemented on FreeBSD.

Hmm, well, seems on Linux the displaced stepping thing just reuses a block of instructions at the entry point assuming they won't get re-used once the program has started execution.

[kib]

Any further opinions on this? Could anybody review the patch, please?

[markj]

lib/libc/sys/ptrace.2

130

132

135

136

138

140

(I think "legitimate" is redundant.)

142

143

1004

1010

1014

1016

1049

Maybe note specifically that the process can deadlock. In this situation, the only way out is to kill the process.

sys/kern/kern_sig.c

2674

2682

It's a corner case, not very important. Maybe this behaviour is actually preferable. I would perhaps add a comment noting that trap_enotcap (and the corresponding procctl() flag) are ignored in this mode.

2725

The amount of duplication is still worrisome to me. I still don't have a concrete suggestion to resolve it though.

2752

As a matter of naming, we have _remotereq, _syscallrq and _coredump. IMHO it'd be nicer to be more consistent and just use req as the suffix everywhere, also for TDB_ flags. rq is somewhat cryptic to me.

2760

Should we assert that the flag is still set here?

sys/kern/sys_process.c

1646

Why exactly is this assertion true? What if two threads in the tracing process both issue a remote syscall request?

sys/sys/ptrace.h

200

Should we use register_t instead of u_long to represent parameters? That would be consistent with struct syscall_args, and the memcpy in the PT_SC_REMOTE effectively assumes that the types are compatible already.

[kib]

sys/kern/sys_process.c

1646

When I added PT_COREDUMP, I made ptrace(2) block other requests until the current one finished. Please see P2_PTRACEREQ code near the (long) beginning of kern_ptrace().

sys/sys/ptrace.h

200

I changed the type to syscallarg_t.

[kib]

RQ->REQ
u_long->syscallarg_t
Editing of the man page changes

[markj]

lib/libc/sys/ptrace.2

1057