This is one of the pieces required to make modern (ie Focal)
strace(1) work.
Sponsored by: The FreeBSD Foundation
Details
Details
- Reviewers
jhb - Group Reviewers
Linux Emulation - Commits
- rG8371bf67d632: linux: implement PTRACE_GET_SYSCALL_INFO
rGbdf0f24bb16d: linux: implement PTRACE_GET_SYSCALL_INFO
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| 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 |
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(). | |
| 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). | |
| 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? | |
Comment Actions
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?
Comment Actions
You can edit lib/libsysdecode/mktables to exclude it; the optional fourth argument to gen_table is a regex passed to egrep -v.