On Tue, Feb 28, 2012 at 10:43 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > On 02/27, Will Drewry wrote: >> On Mon, Feb 27, 2012 at 11:54 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote: >> > On 02/24, Will Drewry wrote: >> >> >> >> arch/Kconfig | 1 + >> >> include/linux/ptrace.h | 7 +++++-- >> >> include/linux/seccomp.h | 4 +++- >> >> include/linux/tracehook.h | 6 ++++++ >> >> kernel/ptrace.c | 4 ++++ >> >> kernel/seccomp.c | 18 ++++++++++++++++++ >> > >> > FYI, this conflicts with the changes -mm tree. >> > >> > The changes in ptrace.* confict with Denys's >> > "ptrace: simplify PTRACE_foo constants and PTRACE_SETOPTIONS code" >> > >> > The change in tracehook.h conflicts with >> > "ptrace: the killed tracee should not enter the syscall" >> >> What's the best way to reconcile this in this day and age? > > Of course I'd prefer if you make this change on top of Denys's patch ;) > > Besides, if you agree with PTRACE_EVENT_SECCOMP/PTRACE_O_SECCOMP you > need only one trivial change in ptrace.h. I think that works quite well :) >> I don't see >> these in kernel-next yet and I can't tell if there is a public -mm >> anywhere anymore. > > Strange... I didn't check, but every patch in > http://marc.info/?l=linux-mm-commits has this note: > > The -mm tree is included into linux-next and is updated > there every 3-4 working days It appears to have been pulled in ~8 hours ago. I'm rebasing to next now. >> >> --- a/kernel/seccomp.c >> >> +++ b/kernel/seccomp.c >> >> @@ -354,6 +354,24 @@ int __secure_computing_int(int this_syscall) >> >> seccomp_send_sigsys(this_syscall, reason_code); >> >> return -1; >> >> } >> >> + case SECCOMP_RET_TRACE: { >> >> + int ret; >> >> + struct pt_regs *regs = task_pt_regs(current); >> >> + if (!(test_tsk_thread_flag(current, TIF_SYSCALL_TRACE)) || >> >> + !(current->ptrace & PT_TRACE_SECCOMP)) >> >> + return -1; >> >> + /* >> >> + * PT_TRACE_SECCOMP and seccomp.trace indicate whether >> >> + * tracehook_report_syscall_entry needs to signal the >> >> + * tracer. This avoids race conditions in hand off and >> >> + * the requirement for TIF_SYSCALL_TRACE ensures that >> >> + * we are in the syscall slow path. >> >> + */ >> >> + current->seccomp.trace = 1; >> >> + ret = tracehook_report_syscall_entry(regs); >> >> + current->seccomp.trace = 0; >> >> + return ret; >> > >> > To be honest, this interface looks a bit strange to me... >> > >> > Once again, sorry if this was already discussed. But perhaps it would >> > be better to introduce PTRACE_EVENT_SECCOMP/PTRACE_O_SECCOMP instead? >> > >> > SECCOMP_RET_TRACE: could simply do ptrace_event(PTRACE_EVENT_SECCOMP) >> > unconditionaly. The tracer can set the option and do PTRACE_CONT if it >> > doesn't want the system call notifications. >> >> Works for me - this also gets rid of the extra int for brief state >> tracking. I'll switch over to that in the next rev. > > Great. In this case this patch becomes really trivial. Just 2 defines > in ptrace.h and the unconditional ptrace_event() under SECCOMP_RET_TRACE. > > But probably you should check fatal_signal_pending(current) after > ptrace_event() returns, ptrace_event() returns void. Ah yeah - I don't want to reintroduce that issue :) Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
