2008-08-25 07:37:42

by Pierre Morel

[permalink] [raw]
Subject: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

Subject: [PATCH] system call notification with self_ptrace

From: Pierre Morel <[email protected]>


PTRACE SELF

This patch adds a new functionality to ptrace: system call notification to
the current process.
When a process requests self ptrace, with the new request PTRACE_SELF_ON:

1. the next system call performed by the process will not be executed
2. self ptrace will be disabled for the process
3. a SIGSYS signal will be sent to the process.

With an appropriate SIGSYS signal handler, the process can access its own
data structures to

1. get the system call number from the siginfo structure
2. get the system call arguments from the stack
3. instrument the system call with other system calls
4. emulate the system call with other system calls
5. change the arguments of the system call
6. perform the system call for good
7. change the return value of the system call
8. request self ptrace again before returning.

The new request PTRACE_SELF_OFF disables self ptrace.


Signed-off-by: Pierre Morel <[email protected]>
Signed-off-by: Volker Sameske <[email protected]>
---

arch/s390/kernel/ptrace.c | 18 ++++++++++++++++++
arch/s390/kernel/signal.c | 5 +++++
arch/x86/kernel/ptrace.c | 36 ++++++++++++++++++++++++++++++++++++
arch/x86/kernel/signal_32.c | 5 +++++
arch/x86/kernel/signal_64.c | 5 +++++
include/asm-generic/siginfo.h | 6 ++++++
include/linux/ptrace.h | 5 +++++
kernel/ptrace.c | 17 +++++++++++++++++
8 files changed, 97 insertions(+)

Index: linux/arch/s390/kernel/ptrace.c
===================================================================
--- linux.orig/arch/s390/kernel/ptrace.c
+++ linux/arch/s390/kernel/ptrace.c
@@ -583,6 +583,24 @@ syscall_trace(struct pt_regs *regs, int

if (!test_thread_flag(TIF_SYSCALL_TRACE))
goto out;
+
+ if ((current->ptrace & PT_SELF)
+ && (regs->gprs[2] != __NR_rt_sigreturn)
+ && (regs->gprs[2] != __NR_ptrace)) {
+ if (!entryexit) {
+ struct siginfo info;
+
+ memset(&info, 0, sizeof(struct siginfo));
+ info.si_signo = SIGSYS;
+ info.si_code = SYS_SYSCALL;
+ info.si_errno = regs->gprs[2];
+ info.si_addr = (void *)regs->orig_gpr2;
+ send_sig_info(SIGSYS, &info, current);
+ regs->gprs[2] = -1;
+ }
+ return;
+ }
+
if (!(current->ptrace & PT_PTRACED))
goto out;
ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
Index: linux/arch/s390/kernel/signal.c
===================================================================
--- linux.orig/arch/s390/kernel/signal.c
+++ linux/arch/s390/kernel/signal.c
@@ -409,6 +409,11 @@ handle_signal(unsigned long sig, struct
spin_unlock_irq(&current->sighand->siglock);
}

+ if (current->ptrace & PT_SELF) {
+ clear_thread_flag(TIF_SYSCALL_TRACE);
+ current->ptrace &= ~PT_SELF;
+ }
+
return ret;
}

Index: linux/arch/x86/kernel/ptrace.c
===================================================================
--- linux.orig/arch/x86/kernel/ptrace.c
+++ linux/arch/x86/kernel/ptrace.c
@@ -20,6 +20,7 @@
#include <linux/audit.h>
#include <linux/seccomp.h>
#include <linux/signal.h>
+#include <linux/unistd.h>

#include <asm/uaccess.h>
#include <asm/pgtable.h>
@@ -1394,6 +1395,21 @@ int do_syscall_trace(struct pt_regs *reg
if (!entryexit)
secure_computing(regs->orig_ax);

+ if ((current->ptrace & PT_SELF)
+ && (regs->orig_ax != __NR_rt_sigreturn)
+ && (regs->orig_ax != __NR_ptrace)) {
+ if (!entryexit) {
+ 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. */
+ }
+
if (unlikely(current->audit_context)) {
if (entryexit)
audit_syscall_exit(AUDITSC_RESULT(regs->ax),
@@ -1486,6 +1502,20 @@ asmlinkage void syscall_trace_enter(stru
/* do the secure computing check first */
secure_computing(regs->orig_ax);

+ if ((current->ptrace & PT_SELF)
+ && (regs->orig_rax != __NR_rt_sigreturn)
+ && (regs->orig_rax != __NR_ptrace)) {
+ struct siginfo info;
+
+ memset(&info, 0, sizeof(struct siginfo));
+ info.si_signo = SIGSYS;
+ info.si_code = SYS_SYSCALL;
+ info.si_addr = (void *) regs->orig_rax;
+ send_sig_info(SIGSYS, &info, current);
+ regs->rax = -1 ;
+ return; /* Skip system call, deliver signal. */
+ }
+
if (test_thread_flag(TIF_SYSCALL_TRACE)
&& (current->ptrace & PT_PTRACED))
syscall_trace(regs);
@@ -1507,6 +1537,12 @@ asmlinkage void syscall_trace_enter(stru

asmlinkage void syscall_trace_leave(struct pt_regs *regs)
{
+ if ((current->ptrace & PT_SELF)
+ && (regs->orig_rax != __NR_rt_sigreturn)
+ && (regs->orig_rax != __NR_ptrace)) {
+ regs->rax = -1 ;
+ return; /* Skip system call. */
+ }
if (unlikely(current->audit_context))
audit_syscall_exit(AUDITSC_RESULT(regs->ax), regs->ax);

Index: linux/arch/x86/kernel/signal_32.c
===================================================================
--- linux.orig/arch/x86/kernel/signal_32.c
+++ linux/arch/x86/kernel/signal_32.c
@@ -94,6 +94,11 @@ sys_sigaction(int sig, const struct old_
__put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask);
}

+ if (current->ptrace & PT_SELF) {
+ clear_thread_flag(TIF_SYSCALL_TRACE);
+ current->ptrace &= ~PT_SELF;
+ }
+
return ret;
}

Index: linux/arch/x86/kernel/signal_64.c
===================================================================
--- linux.orig/arch/x86/kernel/signal_64.c
+++ linux/arch/x86/kernel/signal_64.c
@@ -402,6 +402,11 @@ handle_signal(unsigned long sig, siginfo
spin_unlock_irq(&current->sighand->siglock);
}

+ if (current->ptrace & PT_SELF) {
+ clear_thread_flag(TIF_SYSCALL_TRACE);
+ current->ptrace &= ~PT_SELF;
+ }
+
return ret;
}

Index: linux/include/asm-generic/siginfo.h
===================================================================
--- linux.orig/include/asm-generic/siginfo.h
+++ linux/include/asm-generic/siginfo.h
@@ -224,6 +224,12 @@ typedef struct siginfo {
#define NSIGPOLL 6

/*
+ * SIGSYS si_codes
+ */
+#define SYS_SYSCALL (__SI_FAULT|1) /* system call notification */
+#define NSIGSYS 1
+
+/*
* sigevent definitions
*
* It seems likely that SIGEV_THREAD will have to be handled from
Index: linux/include/linux/ptrace.h
===================================================================
--- linux.orig/include/linux/ptrace.h
+++ linux/include/linux/ptrace.h
@@ -27,6 +27,10 @@
#define PTRACE_GETSIGINFO 0x4202
#define PTRACE_SETSIGINFO 0x4203

+/* PTRACE SELF requests */
+#define PTRACE_SELF_ON 0x4281
+#define PTRACE_SELF_OFF 0x4282
+
/* options set using PTRACE_SETOPTIONS */
#define PTRACE_O_TRACESYSGOOD 0x00000001
#define PTRACE_O_TRACEFORK 0x00000002
@@ -67,6 +71,7 @@
#define PT_TRACE_EXEC 0x00000080
#define PT_TRACE_VFORK_DONE 0x00000100
#define PT_TRACE_EXIT 0x00000200
+#define PT_SELF 0x00000400

#define PT_TRACE_MASK 0x000003f4

Index: linux/kernel/ptrace.c
===================================================================
--- linux.orig/kernel/ptrace.c
+++ linux/kernel/ptrace.c
@@ -550,6 +550,23 @@ asmlinkage long sys_ptrace(long request,
goto out;
}

+ if (request == PTRACE_SELF_ON) {
+ task_lock(current);
+ set_thread_flag(TIF_SYSCALL_TRACE);
+ current->ptrace |= PT_SELF;
+ task_unlock(current);
+ ret = 0;
+ goto out;
+ }
+ if (request == PTRACE_SELF_OFF) {
+ task_lock(current);
+ clear_thread_flag(TIF_SYSCALL_TRACE);
+ current->ptrace &= ~PT_SELF;
+ task_unlock(current);
+ ret = 0;
+ goto out;
+ }
+
child = ptrace_get_task_struct(pid);
if (IS_ERR(child)) {
ret = PTR_ERR(child);


Attachments:
self_ptrace_08.patch (7.50 kB)

2008-08-25 16:33:27

by Dave Hansen

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

On Mon, 2008-08-25 at 09:34 +0200, Pierre Morel wrote:
> + if ((current->ptrace & PT_SELF)
> + && (regs->orig_ax != __NR_rt_sigreturn)
> + && (regs->orig_ax != __NR_ptrace)) {
> + if (!entryexit) {
> + 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

2008-08-25 16:49:32

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

On 08/25, Pierre Morel wrote:
>
> @@ -550,6 +550,23 @@ asmlinkage long sys_ptrace(long request,
> goto out;
> }
>
> + 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.

2008-08-26 12:36:54

by Pierre Morel

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

Hi,

Dave Hansen wrote:
> On Mon, 2008-08-25 at 09:34 +0200, Pierre Morel wrote:
>
>> + if ((current->ptrace & PT_SELF)
>> + && (regs->orig_ax != __NR_rt_sigreturn)
>> + && (regs->orig_ax != __NR_ptrace)) {
>> + if (!entryexit) {
>> + 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.
>
You are right, I will rework the patch and send it again, it has a lot of
formating errors indeed.
> 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.
>
Yes, thank you it is a good tip.
> 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.
>
Yes, you are right too, I will rework the patch description too.
> -- Dave
>
>
Thanks for the comments, I rework the patch.

Pierre

--
=============
Pierre Morel
RTOS and Embedded Linux

2008-08-26 14:08:29

by Pierre Morel

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

Hello Oleg,

Oleg Nesterov wrote:
> On 08/25, Pierre Morel wrote:
>
>> @@ -550,6 +550,23 @@ asmlinkage long sys_ptrace(long request,
>> goto out;
>> }
>>
>> + 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).
>
Yes it is the way it is intended to work.
PT_SELF and other ptrace requests are not correlated,
I use the ptrace infrastructure to take advantage
of the existing system call interception framework.
> 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

The second solution seems easier, simpler and more
readable but extends the task struct.

What do you think is the best way to do it?

> Oleg.
>
>
Thanks,

Pierre




--
=============
Pierre Morel
RTOS and Embedded Linux

2008-08-26 16:23:24

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

On 08/26, Pierre Morel wrote:
>
> Oleg Nesterov wrote:
> >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

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.

2008-08-27 14:36:57

by Pierre Morel

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

Oleg Nesterov wrote:
> On 08/26, Pierre Morel wrote:
>
>> Oleg Nesterov wrote:
>>
>>> 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
>>
>
> 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().
>
Me neither, it should be only in handle_signal(), sorry it is a bug.
I am reworking the patch to take your remarks and the remarks
of Dave into account.
> 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.
> 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.
>
Yes.
The situation is the following: the ptrace_self implementation is not
compatible with the standard ptrace.
In fact it is a new tracing system using the infrastructure of
ptrace because it exist but it could leave completely separate from
ptrace.
> 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.
>
Yes, I will had exclusive access to the tracing so that one can not
use both ptrace and self_ptrace for the same process.
>
> 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.
> Oleg.
>
>
thanks,

Pierre

--
=============
Pierre Morel
RTOS and Embedded Linux

2008-08-27 16:20:46

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

On 08/27, Pierre Morel wrote:
>
> Oleg Nesterov wrote:
>
> >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.

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".

> >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.

Ah, I forgot to read the changelog, sorry.

Oleg.

2008-08-28 12:07:26

by Pierre Morel

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

Oleg Nesterov wrote:
> On 08/27, Pierre Morel wrote:
>
>> Oleg Nesterov wrote:
>>
>>
>>> 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.
>
It is right but brings the overhead of a 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".
>

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.

I see this more like a security, the "bouncing" feature
is only enabled until next syscall or signal, never more.

This instrumentation method allows with this little patch to do
all the syscall and signal instrumentation in userland and inside the
address space of the instrumented application.

I expect we will have a big improvement of instrumenting tools like
- debugger, tracing tool,
- virtualization applications like UML
- High availability: checkpoint and restart, record and replay.
because of the reduction of IPC and task switch overhead.

Pierre

--
=============
Pierre Morel
RTOS and Embedded Linux

2008-08-28 12:28:36

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

On 08/28, Pierre Morel wrote:
>
> Oleg Nesterov wrote:
> >On 08/27, Pierre Morel wrote:
> >
> >>Oleg Nesterov wrote:
> >>
> >>
> >>>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.
> >
> It is right but brings the overhead of a syscall.

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

> >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.

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.

Oleg.

2008-08-28 13:29:32

by Pierre Morel

[permalink] [raw]
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

Oleg Nesterov wrote:
> On 08/28, Pierre Morel wrote:
>
>> Oleg Nesterov wrote:
>>
>>> On 08/27, Pierre Morel wrote:
>>>
>>>
>>>> Oleg Nesterov wrote:
>>>>
>>>>
>>>>
>>>>> 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.
>>>
>>>
>> It is right but brings the overhead of a syscall.
>>
>
> Well, this overhead is very small compared to the signal delivery.
>
May be, but very high compared to the operation to just
clear a flag in the task struct.
This operation must be done anyway.
>
>>> 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.
>>
>
> 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.
>
ok, please read "set PTRACE_SELF_ON"
where I wrote "reset PTRACE_SELF_ON" above.

Now, suppose the application does the following:

sigsys_handler(sig)
{
....
ptrace(PTRACE_SELF_ON);
}

sigx_handler(sig)
{
....
ptrace(PTRACE_SELF_ON);
if (sig_has_a_handler)
call_the_handler()
ptrace(PTRACE_SELF_OFF);
...
ptrace(PTRACE_SELF_ON);
}

main(){
...
signal(SIGSYS, sigsys_handler);
for(i=0; i<MAXSIGS; i++)
signal(i, sigx_handler);
ptrace(PTRACE_SELF_ON);
jump_into_instrumented_code;
...
}

If the instrumented code ever does a call to
signal(2), the call will be received by sig_sys() handler,
where the call the application signal handler can be performed with
PTRACE_SELF_ON.
This implies appropriate instrumentation of signal(2) and sigaction(2).

Pierre

--
=============
Pierre Morel
RTOS and Embedded Linux