Changeset View
Standalone View
sys/kern/sys_process.c
Show First 20 Lines • Show All 481 Lines • ▼ Show 20 Lines | sys_ptrace(struct thread *td, struct ptrace_args *uap) | ||||
AUDIT_ARG_PID(uap->pid); | AUDIT_ARG_PID(uap->pid); | ||||
AUDIT_ARG_CMD(uap->req); | AUDIT_ARG_CMD(uap->req); | ||||
AUDIT_ARG_VALUE(uap->data); | AUDIT_ARG_VALUE(uap->data); | ||||
addr = &r; | addr = &r; | ||||
switch (uap->req) { | switch (uap->req) { | ||||
case PT_GET_EVENT_MASK: | case PT_GET_EVENT_MASK: | ||||
case PT_LWPINFO: | case PT_LWPINFO: | ||||
case PT_GET_SC_ARGS: | case PT_GET_SC_ARGS: | ||||
case PT_GET_SC_RET: | case PT_GET_SC_RET: | ||||
jhb: I think this should not be enabled for ptrace() for native binaries, only an internal operation… | |||||
break; | break; | ||||
case PT_GETREGS: | case PT_GETREGS: | ||||
bzero(&r.reg, sizeof(r.reg)); | bzero(&r.reg, sizeof(r.reg)); | ||||
break; | break; | ||||
case PT_GETFPREGS: | case PT_GETFPREGS: | ||||
bzero(&r.fpreg, sizeof(r.fpreg)); | bzero(&r.fpreg, sizeof(r.fpreg)); | ||||
break; | break; | ||||
case PT_GETDBREGS: | case PT_GETDBREGS: | ||||
▲ Show 20 Lines • Show All 420 Lines • ▼ Show 20 Lines | case PT_GET_SC_ARGS: | ||||
if ((td2->td_dbgflags & (TDB_SCE | TDB_SCX)) == 0 | if ((td2->td_dbgflags & (TDB_SCE | TDB_SCX)) == 0 | ||||
#ifdef COMPAT_FREEBSD32 | #ifdef COMPAT_FREEBSD32 | ||||
|| (wrap32 && !safe) | || (wrap32 && !safe) | ||||
#endif | #endif | ||||
) { | ) { | ||||
error = EINVAL; | error = EINVAL; | ||||
break; | break; | ||||
} | } | ||||
bzero(addr, sizeof(td2->td_sa.args)); | bcopy(td2->td_sa.args, addr, sizeof(td2->td_sa.args)); | ||||
jhbUnsubmitted Not Done Inline ActionsThis 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. jhb: This is a regression I believe as you would potentially leak earlier syscall arguments (albeit… | |||||
traszAuthorUnsubmitted Done Inline ActionsThe 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)? trasz: The reason for this chunk is that's what's expected by Linux strace(1).
What it does is, at… | |||||
jhbUnsubmitted Not Done Inline Actions
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(). jhb: > The reason for this chunk is that's what's expected by Linux strace(1).
>
> What it does is… | |||||
jrtc27Unsubmitted Not Done Inline ActionsDepends 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). jrtc27: Depends what architecture you're on (i.e. whether some syscall arguments go on the stack). If… | |||||
bcopy(td2->td_sa.args, addr, td2->td_sa.callp->sy_narg * | |||||
sizeof(register_t)); | |||||
break; | break; | ||||
case PT_GET_SC_RET: | case PT_GET_SC_RET: | ||||
if ((td2->td_dbgflags & (TDB_SCX)) == 0 | if ((td2->td_dbgflags & (TDB_SCX)) == 0 | ||||
#ifdef COMPAT_FREEBSD32 | #ifdef COMPAT_FREEBSD32 | ||||
|| (wrap32 && !safe) | || (wrap32 && !safe) | ||||
#endif | #endif | ||||
) { | ) { | ||||
▲ Show 20 Lines • Show All 389 Lines • Show Last 20 Lines |
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.