Page MenuHomeFreeBSD

linux: implement PTRACE_GET_SYSCALL_INFO
ClosedPublic

Authored by trasz on Jan 17 2021, 6:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 16, 4:41 AM
Unknown Object (File)
Thu, Apr 11, 5:48 PM
Unknown Object (File)
Tue, Apr 9, 8:19 PM
Unknown Object (File)
Tue, Apr 9, 3:11 AM
Unknown Object (File)
Mon, Apr 8, 2:04 PM
Unknown Object (File)
Wed, Apr 3, 9:08 AM
Unknown Object (File)
Mar 17 2024, 5:02 PM
Unknown Object (File)
Mar 16 2024, 9:56 AM
Subscribers

Details

Summary

This is one of the pieces required to make modern (ie Focal)
strace(1) work.

Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

trasz requested review of this revision.Jan 17 2021, 6:17 PM
sys/amd64/linux/linux_ptrace.c
96–98

Are these MD?

sys/amd64/linux/linux_ptrace.c
96–98

I'm guessing they are not, but for now all of amd64 implementation of ptrace is in a MD-file. This will probably change once it gets ported to arm64.

sys/amd64/linux/linux_ptrace.c
571

This should never occur as FreeBSD doesn't use negative errno values. I think instead you should make the previous 'sr.sr_error > 0' into an else as the only conditions are 'sr.sr_error == 0' and 'sr.sr_error != 0'.

580

I would move the kern_ptrace of PT_REG down to here where you actually use it. Presumably, this block to set ip/sp would end up being MD, but the rest of this function is MI and should be shared across platforms.

sys/kern/sys_process.c
1015

This is a regression I believe as you would potentially leak earlier syscall arguments (albeit from the same thread). Certainly it is not part of this commit and would need to be its own commit, but I don't really think it's a good idea to change.

sys/amd64/linux/linux_ptrace.c
571

Thanks for clarifying this.

580

I agree about the block placement. I'd prefer not splitting this into MI/MD chunks just yet, there might be some unexplained differences in operation between versions - like with syscall numbers in Linux.

sys/kern/sys_process.c
1015

The reason for this chunk is that's what's expected by Linux strace(1).

What it does is, at initialization it tests whether ptrace works as expected; in this case it calls... I think close(2), or some other single-argument syscall, _with six arguments_, and then verifies whether it can fetch them using this API; otherwise it just bails out.

How much of a problem is this leak? Do we always clear the arguments on, say, execve(2)?

sys/kern/sys_process.c
1015

The reason for this chunk is that's what's expected by Linux strace(1).

What it does is, at initialization it tests whether ptrace works as expected; in this case it calls... I think close(2), or some other single-argument syscall, _with six arguments_, and then verifies whether it can fetch them using this API; otherwise it just bails out.

How much of a problem is this leak? Do we always clear the arguments on, say, execve(2)?

That's horrible. What horrible code. I don't want to break perfectly good FreeBSD code to cater to such nonsense. You can either add a private PT_GET_SC_ARGS_RAW for Linux to use that does this, or handle this directly in the Linux code by reading from td_sa without using kern_ptrace().

jrtc27 added inline comments.
sys/kern/sys_process.c
1015

Depends what architecture you're on (i.e. whether some syscall arguments go on the stack). If everything is passed in registers we just copy the whole trapframe's worth, so you'd "just" be leaking arguments that happened to be in the later argument registers, not things from way earlier that the tracee no longer even necessarily has (well, except for syscall(2) itself, that ends up copying one fewer argument, so if that was the last syscall then you'll leak the previous syscall's last register).

Fix problems reported by jhb@, also style(9).

sys/kern/sys_process.c
492

I think this should not be enabled for ptrace() for native binaries, only an internal operation for Linux ABI to use. That means though that sys_ptrace() has to explicitly fail this request here with EINVAL. freebsd32_ptrace() also needs updating to fail this request.

sys/sys/ptrace.h
89

Perhaps #ifdef _KERNEL around this?

trasz marked 2 inline comments as done.

Handle freebsd32.

This revision is now accepted and ready to land.Sep 8 2021, 6:21 PM

There's still one problem: the #ifdef _KERNEL breaks world build (lib/libsysdecode/tables.h:406:13: error: use of undeclared identifier 'PT_GET_SC_ARGS_ALL'). Is there already some #define to allow libsysdecode to use kernel includes, or is there a different way?

There's still one problem: the #ifdef _KERNEL breaks world build (lib/libsysdecode/tables.h:406:13: error: use of undeclared identifier 'PT_GET_SC_ARGS_ALL'). Is there already some #define to allow libsysdecode to use kernel includes, or is there a different way?

You can edit lib/libsysdecode/mktables to exclude it; the optional fourth argument to gen_table is a regex passed to egrep -v.

This revision now requires review to proceed.Sep 13 2021, 1:30 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 14 2021, 8:42 PM
This revision was automatically updated to reflect the committed changes.