System call notification with self_ptrace

arch/x86/kernel/ptrace.c +4 -0

+       if ((current->ptrace & PT_SELF)
+               && (regs->orig_ax != __NR_rt_sigreturn)
+               && (regs->orig_ax != __NR_ptrace)) {
+               if (!entryexit) {

Quoted 5 lines · Pierre Morel

arch/x86/kernel/ptrace.c +4 -0

+       if ((current->ptrace & PT_SELF)
+               && (regs->orig_ax != __NR_rt_sigreturn)
+               && (regs->orig_ax != __NR_ptrace)) {
+               if (!entryexit) {

arch/x86/kernel/ptrace.c int do_syscall_trace(struct pt_regs *reg · line 1411

Quoted 10 lines

arch/x86/kernel/ptrace.c +10 -0

+               struct siginfo info;
+
+               memset(&info, 0, sizeof(struct siginfo));
+               info.si_signo = SIGSYS;
+               info.si_code = SYS_SYSCALL;
+               info.si_addr = (void *) regs->orig_ax;
+               send_sig_info(SIGSYS, &info, current);
+               }
+               return 1; /* Skip system call, deliver signal. */
+       }

The indenting here looks messed up.

Also, there looks to be a pretty substantial amount of copy-and-paste code in those little if()s. It's only going to get worse as we add more architectures. If there's ever a little buglet in that bit of code, or we need to tweak it it some way, it'll be a bitch to fix.

For instance, if you have a little arch-independent helper like this:

static inline int is_self_ptracing(unsigned long syscall_reg)

{
	if (!(current->ptrace & PT_SELF))
		return 0;
	if (syscall_reg == __NR_rt_sigreturn)
		return 0;
	if (syscall_reg == __NR_ptrace)
		return 0;
	return 1;
}

You can call it like this:

	if (is_self_ptracing(regs->gprs[2]))
...
	if (is_self_ptracing(regs->orig_ax))
...
	if (is_self_ptracing(regs->orig_rax))

Something similar can probably be done for the siginfo construction.

You should basically try and think of ways to abstract this stuff every single time you touch arch code.

Why don't you also mention why you really want this feature. That's missing from the description.

-- Dave

Quoted 4 lines · Pierre Morel

kernel/ptrace.c +0 -0

@@ -550,6 +550,23 @@ asmlinkage long sys_ptrace(long request,
 		goto out;
 	}
 

kernel/ptrace.c asmlinkage long sys_ptrace(long request,

Quoted 5 lines

kernel/ptrace.c +4 -0

+	if (request == PTRACE_SELF_ON) {
+		task_lock(current);
+		set_thread_flag(TIF_SYSCALL_TRACE);
+		current->ptrace |= PT_SELF;
 

I didn't read the whole patch, but this sets PT_SELF without PT_PTRACED (and without ptrace_attach).

We have some "->ptrace != 0" checks which can misunderstand this. Just for example, suppose that the task does sys_ptrace(PTRACE_SELF_ON) and then its parent dies. I guess in that case forget_original_parent() will hit BUG_ON(p->ptrace), no?

Oleg.

Quoted 6 lines · Oleg Nesterov

We have some "->ptrace != 0" checks which can misunderstand this. Just for example, suppose that the task does sys_ptrace(PTRACE_SELF_ON) and then its parent dies. I guess in that case forget_original_parent() will hit BUG_ON(p->ptrace), no?

Yes you are right, I will take care of those cases. I have the choice between:

- tracking all references to the ptrace flags and add a test for PT_SELF or a mask.

- add a dedicated task_struct entry to hold the PT_SELF flag

Quoted 15 lines · Pierre Morel

Quoted 7 lines · Oleg Nesterov

We have some "->ptrace != 0" checks which can misunderstand this. Just for example, suppose that the task does sys_ptrace(PTRACE_SELF_ON) and then its parent dies. I guess in that case forget_original_parent() will hit BUG_ON(p->ptrace), no?

Yes you are right, I will take care of those cases. I have the choice between:

- tracking all references to the ptrace flags and add a test for PT_SELF or a mask.

Quoted 2 lines

- add a dedicated task_struct entry to hold the PT_SELF flag

Well, given that PT_SELF is exotic, neither choice looks very good, imho. But I am not expert and maintainer is cc'ed ;)

I don't understand why this patch changes the x86's sys_sigaction().

On s390 the patch changes handle_signal(), this is not clear to me too.

do_syscall_trace() filters out __NR_ptrace, this afaics means that the handler for SIGSYS can happily call sys_ptrace(PTRACE_SELF_OFF) and clear PT_SELF/TIF_SYSCALL_TRACE.

I must admit, personally I don't think the whole idea is good... And what if the user of PT_SELF is ptraced? The usage of TIF_SYSCALL_TRACE doesn't look "safe" in that case.

Isn't it possible to implement this behaviour in the user space? If the task needs the PT_SELF behaviour, it can fork another process which will do PTRACE_ATTACH and then send the notifications to the task. We can use signals or something else.

Oleg.

Quoted 2 lines · Oleg Nesterov

On s390 the patch changes handle_signal(), this is not clear to me too.

The patch clears the trace flags before delivering the signal so that the signal handler can use system call without bouncing again.

Quoted 5 lines

Isn't it possible to implement this behaviour in the user space? If the task needs the PT_SELF behaviour, it can fork another process which will do PTRACE_ATTACH and then send the notifications to the task. We can use signals or something else.

In this case we would go back to standard ptrace behaviour. The goal of the patch is to avoid the overhead of task switching and IPC when instrumenting the process.

Quoted 6 lines · Pierre Morel

Quoted 3 lines · Oleg Nesterov

On s390 the patch changes handle_signal(), this is not clear to me too.

Quoted 3 lines

The patch clears the trace flags before delivering the signal so that the signal handler can use system call without bouncing again.

Yes I see. But the signal handler for SIGSYS can fisrt do sys_ptrace(PTRACE_SELF_OFF) (which is filtered out), and then use any other syscall.

With this patch PT_SELF is cleared on any signal. This doesn't look right. Let's suppose that another signal comes in parallel with SIGSYS. It is very possible that the handler for that another signal will be called first, this handler can do some syscall which will be "missed".

Quoted 12 lines · Oleg Nesterov

Quoted 9 lines · Pierre Morel

Quoted 3 lines · Oleg Nesterov

On s390 the patch changes handle_signal(), this is not clear to me too.

The patch clears the trace flags before delivering the signal so that the signal handler can use system call without bouncing again.

Quoted 4 lines

Yes I see. But the signal handler for SIGSYS can fisrt do sys_ptrace(PTRACE_SELF_OFF) (which is filtered out), and then use any other syscall.

It is right but brings the overhead of a syscall.

Quoted 5 lines

With this patch PT_SELF is cleared on any signal. This doesn't look right. Let's suppose that another signal comes in parallel with SIGSYS. It is very possible that the handler for that another signal will be called first, this handler can do some syscall which will be "missed".

If the tracing application catches all signals before delivering them to the instrumented original handler there is no problem, the catching code can reset PTRACE_SELF_ON before calling the instrumented application's original handler.
The instrumented code will then bounce as expected.

Quoted 22 lines · Pierre Morel

Quoted 20 lines · Oleg Nesterov

Quoted 4 lines · Pierre Morel

Oleg Nesterov wrote:

Quoted 8 lines

Quoted 4 lines

On s390 the patch changes handle_signal(), this is not clear to me too.

The patch clears the trace flags before delivering the signal so that the signal handler can use system call without bouncing again.

Yes I see. But the signal handler for SIGSYS can fisrt do sys_ptrace(PTRACE_SELF_OFF) (which is filtered out), and then use any other syscall.

Quoted 2 lines

It is right but brings the overhead of a syscall.

Well, this overhead is very small compared to the signal delivery.

Quoted 7 lines

Quoted 7 lines

With this patch PT_SELF is cleared on any signal. This doesn't look right. Let's suppose that another signal comes in parallel with SIGSYS. It is very possible that the handler for that another signal will be called first, this handler can do some syscall which will be "missed".

Quoted 6 lines

If the tracing application catches all signals before delivering them to the instrumented original handler there is no problem, the catching code can reset PTRACE_SELF_ON before calling the instrumented application's original handler.
The instrumented code will then bounce as expected.

Sorry, can't understand the text above :(

OK, let's suppose the application does

	ptrace(PTRACE_SELF_ON);
	...
	syscall();

This "syscall()" above should trigger the handler for SIGSYS. But what if another signal (with handler) comes in between? In that case handle_signal() clears PT_SELF/TIF_SYSCALL_TRACE, this syscall() (or any other) doesn't send SIGSYS.