Hmm, do you have plans for a PT_SETREGSET as well? To replace the ARM VFP, and PPC VMX, etc. sets we need set, not just get. Also, as I said previously on IRC I would like to have a PT_GETREGSET_INFO where the iovec in addr points to a structure. The first field would always be a uint64_t size member, but the rest of the fields could vary by note type. For XSTATE the structure would also hold the xcr0 mask to replace PT_GETXSTATE_INFO, so something like:

struct regset_info {
    uint64_t len;
};

and then in x86/include/ptrace.h:

struct xstate_regset_info {
     uint64_t len;
     uint64_t xsave_mask;

If you return whole XSTATE from ptrace(PT_GETREGSET, NT_X86_XSTATE), what is the point of the new API ? It would make much more sense to me if there was an API to get components like YMM/ZMM/PKRU instead of returning whole blob and then requiring the caller to parse it combining XCR0 bitmask with either prior knowledge of the component sizes, or querying the offsets on live machine.

In D19831#425341, @kib wrote:

If you return whole XSTATE from ptrace(PT_GETREGSET, NT_X86_XSTATE), what is the point of the new API ? It would make much more sense to me if there was an API to get components like YMM/ZMM/PKRU instead of returning whole blob and then requiring the caller to parse it combining XCR0 bitmask with either prior knowledge of the component sizes, or querying the offsets on live machine.

Both GDB and lldb are happy to just get the blob. However, we could use this to replace some of the "one-off" register sets we have now like the VFP registers for ARM, the two separate vector register sets for PPC. I plan to add a new register set on x86 for fs_base/gs_base mostly because I need a new core dump note to permit TLS to work in gdb on core dumps. ARM needs the TLS register available in cores and via ptrace for which a new register set would make sense (and/or extending the existing 'struct reg' which PT_GETREGSET makes easier). One thing that you should be able to do, btw is use PT_GETREGSET to fetch the normal and floating point register sets. Floating point is just NT_FPREGSET. struct reg is a bit harder. My guess is you'd let NT_PRSTATUS return struct reg (but not the full core dump note), but we should see what Linux does perhaps for this.

In D19831#425330, @jhb wrote:

Hmm, do you have plans for a PT_SETREGSET as well?

Yes, when I have time to investigate how Linux handles a set with an incorrect length.

To replace the ARM VFP, and PPC VMX, etc. sets we need set, not just get. Also, as I said previously on IRC I would like to have a PT_GETREGSET_INFO where the iovec in addr points to a structure. The first field would always be a uint64_t size member, but the rest of the fields could vary by note type.

What would the length hold if the buffer was too small for the data but large enough for the length? I have an update that will set the iovec length to the expected buffer length when the base is NULL so userspace can query it.

In D19831#425359, @jhb wrote:

One thing that you should be able to do, btw is use PT_GETREGSET to fetch the normal and floating point register sets. Floating point is just NT_FPREGSET. struct reg is a bit harder. My guess is you'd let NT_PRSTATUS return struct reg (but not the full core dump note), but we should see what Linux does perhaps for this.

It looks like Linux does this. It then seems to have a special case for NT_PRSTATUS when building a core dump.

I also return the size when the base is NULL in PT_GETREGSET.

This looks fine to me, modulo compat32 and man page updates.

And compat32 rises interesting question, is ELF_REGSET() interface should be per-ABI ? E.g., do we want to have it specialized for the case when Linux process asks for a registers blob, or linuxolator should decode the values of the argument and re-arrange the FreeBSD native regsets into linuxish structures ?

I've tested on arm64. I still need to test the compat32 code but haven't yet as my current testing is on a machine without 32bit arm support.

Have you considered doing the malloc() of the temp buffer and copyin/copyout inside of proc_read_regset and proc_write_regset? This simplifies sys_ptrace() and the freebsd32 wrapper quite a bit as they simply have to copyin the iovec and don't have to deal with trying to free() the buffer, etc. It also solves your problem of having to worry about malloc()'ing an unvalidated size as you can defer the malloc until after you've located the regset structure and validated the length. You can drop the PROC_LOCK in those two functions to do the malloc + copyin and grab it again when filling the malloc'ed buffer.

Tested on arm64 kernel with both arm64 and armv7 programs calling ptrace with the following parent -> child relationship between the tester and target:

arm64 -> arm64
arm64 -> armv7
armv7 -> armv7

English spelling and grammar in the manual page change LGTM. Can't speak to consistency with the source code changes, or to the correctness of that.

In D19831#719360, @jhb wrote:

You can drop the PROC_LOCK in those two functions to do the malloc + copyin and grab it again when filling the malloc'ed buffer.

Is this safe to do or not? I can revert to Diff 101376 if it is, or Diff 94898 (with man page updates) if not.

In D19831#767144, @andrew wrote:

In D19831#719360, @jhb wrote:

You can drop the PROC_LOCK in those two functions to do the malloc + copyin and grab it again when filling the malloc'ed buffer.

Is this safe to do or not? I can revert to Diff 101376 if it is, or Diff 94898 (with man page updates) if not.

You should be able to safely drop the process lock after the P2_PTRACEREQ flag is set. It blocks other ptrace requests coming in parallel, so neither PT_DETACH nor PT_CONTINUE (to allow the process to exec or exit) can be done under you.

In D19831#767145, @kib wrote:

You should be able to safely drop the process lock after the P2_PTRACEREQ flag is set. It blocks other ptrace requests coming in parallel, so neither PT_DETACH nor PT_CONTINUE (to allow the process to exec or exit) can be done under you.

I'm not worried about other ptrace requests, I was interested in the safty given the following:

I am not sure this is safe. At very least, you could loose the rights to debug the target.

In D19831#767147, @andrew wrote:

In D19831#767145, @kib wrote:

You should be able to safely drop the process lock after the P2_PTRACEREQ flag is set. It blocks other ptrace requests coming in parallel, so neither PT_DETACH nor PT_CONTINUE (to allow the process to exec or exit) can be done under you.

I'm not worried about other ptrace requests, I was interested in the safty given the following:

I am not sure this is safe. At very least, you could loose the rights to debug the target.

You are loosing the rights to debug if either

• PT_DETACH is executed (can only happen from other threads of the debugger)
• the process itself is execing (so e.g. suid situation changes) or exiting

I explained above why neither of the situations can occur after P2_PTRACEREQ is set.

A few nits, but the manual page still LGTM otherwise.