linux: Fix usage of ptrace(PT_GET_SC_ARGS)

[markj]

Summary

The native handler expects the argument to be a pointer to an array of 8
syscall arguments, whereas the emulation provided an array that holds up
to 6.

[kib]

Can we remove the special handling of the linux ABI calls for kern/sys_process.c PT_GET_SC_ARGS?

[markj]

In D50758#1158691, @kib wrote:

Can we remove the special handling of the linux ABI calls for kern/sys_process.c PT_GET_SC_ARGS?

No, I don't think so. Note that the bzero() before that bcopy() in the kern/sys_process.c PT_GET_SC_ARGS is also problematic.

[kib]

In D50758#1159311, @markj wrote:

In D50758#1158691, @kib wrote:

Can we remove the special handling of the linux ABI calls for kern/sys_process.c PT_GET_SC_ARGS?

No, I don't think so. Note that the bzero() before that bcopy() in the kern/sys_process.c PT_GET_SC_ARGS is also problematic.

Sorry but I do not understand. Your patch rightfully pass the array of eight syscallarg_t to the kern_ptrace(). This is the largest array the PT_GET_SC_ARGS case could get at all, so removing the check for SV_PROC_ABI == SV_ABI_LINUX would still work in the bounds of the passed array. And yes, bzero() is equally problematic, but it is not conditional on ABI before (bug), and after (not a bug) your patch.

[markj]

In D50758#1159452, @kib wrote:

In D50758#1159311, @markj wrote:

In D50758#1158691, @kib wrote:

Can we remove the special handling of the linux ABI calls for kern/sys_process.c PT_GET_SC_ARGS?

No, I don't think so. Note that the bzero() before that bcopy() in the kern/sys_process.c PT_GET_SC_ARGS is also problematic.

Sorry but I do not understand. Your patch rightfully pass the array of eight syscallarg_t to the kern_ptrace(). This is the largest array the PT_GET_SC_ARGS case could get at all, so removing the check for SV_PROC_ABI == SV_ABI_LINUX would still work in the bounds of the passed array.

But look at what we do in the SV_PROC_ABI == SV_ABI_LINUX case: we copy all of the argument slots, not just the ones required by the syscall. This has nothing to do with the size of the array. It's so that syscall(SYS_close, 1, 2, 3, 4, 5, 6) reports all six arguments, even though Linux close has only one argument. On FreeBSD, PT_GET_SC_ARGS will only copy one argument for that case, I believe.

[kib]

In D50758#1159512, @markj wrote:

In D50758#1159452, @kib wrote:

In D50758#1159311, @markj wrote:

In D50758#1158691, @kib wrote:

Can we remove the special handling of the linux ABI calls for kern/sys_process.c PT_GET_SC_ARGS?

No, I don't think so. Note that the bzero() before that bcopy() in the kern/sys_process.c PT_GET_SC_ARGS is also problematic.

Sorry but I do not understand. Your patch rightfully pass the array of eight syscallarg_t to the kern_ptrace(). This is the largest array the PT_GET_SC_ARGS case could get at all, so removing the check for SV_PROC_ABI == SV_ABI_LINUX would still work in the bounds of the passed array.

But look at what we do in the SV_PROC_ABI == SV_ABI_LINUX case: we copy all of the argument slots, not just the ones required by the syscall. This has nothing to do with the size of the array. It's so that syscall(SYS_close, 1, 2, 3, 4, 5, 6) reports all six arguments, even though Linux close has only one argument. On FreeBSD, PT_GET_SC_ARGS will only copy one argument for that case, I believe.

Ok, so it is even more wrong, since it assumes that ABI sv_fetch_syscall_args() copied all slots. This is true for amd64 and might be for other arches with register calling conventions, but not for i386.

I think a better thing to do is to stop calling kern_ptrace() at all, but copy from the td.td_sa.args directly.

[markj]

In D50758#1159515, @kib wrote:

In D50758#1159512, @markj wrote:

In D50758#1159452, @kib wrote:

In D50758#1159311, @markj wrote:

In D50758#1158691, @kib wrote:

Can we remove the special handling of the linux ABI calls for kern/sys_process.c PT_GET_SC_ARGS?

No, I don't think so. Note that the bzero() before that bcopy() in the kern/sys_process.c PT_GET_SC_ARGS is also problematic.

Sorry but I do not understand. Your patch rightfully pass the array of eight syscallarg_t to the kern_ptrace(). This is the largest array the PT_GET_SC_ARGS case could get at all, so removing the check for SV_PROC_ABI == SV_ABI_LINUX would still work in the bounds of the passed array.

But look at what we do in the SV_PROC_ABI == SV_ABI_LINUX case: we copy all of the argument slots, not just the ones required by the syscall. This has nothing to do with the size of the array. It's so that syscall(SYS_close, 1, 2, 3, 4, 5, 6) reports all six arguments, even though Linux close has only one argument. On FreeBSD, PT_GET_SC_ARGS will only copy one argument for that case, I believe.

Ok, so it is even more wrong, since it assumes that ABI sv_fetch_syscall_args() copied all slots. This is true for amd64 and might be for other arches with register calling conventions, but not for i386.

Isn't it up to linux_fetch_syscall_args()? It looks like all implementations fetch 6 args.

I think a better thing to do is to stop calling kern_ptrace() at all, but copy from the td.td_sa.args directly.

Wouldn't this require lots of duplicated code? p_candebug() checking etc.

[kib]

In D50758#1159517, @markj wrote:

In D50758#1159515, @kib wrote:

In D50758#1159512, @markj wrote:

In D50758#1159452, @kib wrote:

In D50758#1159311, @markj wrote:

In D50758#1158691, @kib wrote:

Can we remove the special handling of the linux ABI calls for kern/sys_process.c PT_GET_SC_ARGS?

No, I don't think so. Note that the bzero() before that bcopy() in the kern/sys_process.c PT_GET_SC_ARGS is also problematic.

Sorry but I do not understand. Your patch rightfully pass the array of eight syscallarg_t to the kern_ptrace(). This is the largest array the PT_GET_SC_ARGS case could get at all, so removing the check for SV_PROC_ABI == SV_ABI_LINUX would still work in the bounds of the passed array.

But look at what we do in the SV_PROC_ABI == SV_ABI_LINUX case: we copy all of the argument slots, not just the ones required by the syscall. This has nothing to do with the size of the array. It's so that syscall(SYS_close, 1, 2, 3, 4, 5, 6) reports all six arguments, even though Linux close has only one argument. On FreeBSD, PT_GET_SC_ARGS will only copy one argument for that case, I believe.

Ok, so it is even more wrong, since it assumes that ABI sv_fetch_syscall_args() copied all slots. This is true for amd64 and might be for other arches with register calling conventions, but not for i386.

Isn't it up to linux_fetch_syscall_args()? It looks like all implementations fetch 6 args.

I think a better thing to do is to stop calling kern_ptrace() at all, but copy from the td.td_sa.args directly.

Wouldn't this require lots of duplicated code? p_candebug() checking etc.

Yes a lot of code around locking, state checking etc, so the proposal as is is silly.
But I still actively dislike the abuse of PT_GET_SC_ARGS. What about introducing kernel-internal PT_LINUX_GET_SC_INFO that would not be visible in userspace sys/ptrace.h and fitered out in sys_ptrace/freebsd32_ptrace, but only available for direct calls to kern_ptrace? Then it can get whatever semantic is needed for Linux, without messing with FreeBSD-only code.

[kib]

BTW, is there anything that would prevent linux debugger from attaching to FreeBSD-native (or any other non-Linux ABI) process? I did not found such possible check.

[markj]

In D50758#1159522, @kib wrote:

BTW, is there anything that would prevent linux debugger from attaching to FreeBSD-native (or any other non-Linux ABI) process? I did not found such possible check.

No, I don't think so. I tried running Linux strace against a static FreeBSD binary and didn't observe any kernel bugs, the output is nonsense of course. I'm not sure why it is useful to check that though.

[markj]

Try adding a block of Linux-specific ptrace commands, add PT_LINUX_GET_SC_ARGS

[kib]

sys/compat/freebsd32/freebsd32_misc.c

1180

This syntax is not even in C23, but ok.

sys/sys/ptrace.h

93

This can be wrapped into #ifdef _KERNEL as well.

[markj]

Add a new _KERNEL block.

[kib]

Thanks.

[markj]

sys/compat/freebsd32/freebsd32_misc.c

1180

We already use it in several places in the kernel, e.g., vmm.

sys/sys/ptrace.h

93

Actually, this breaks libsysdecode, since mktables pulls in PT_* constants but doesn't define _KERNEL. Maybe these should be called PTLINUX_*? Or we drop the ifdef _KERNEL.

[markj]

Rename constants so that mktables ignores them.