From: Ananth N Mavinakayanahalli on
On Tue, Nov 24, 2009 at 09:01:27PM +0100, Oleg Nesterov wrote:
> Hello.
>
> This is the new iteration of Roland's utrace patch, this time
> with "rewrite-ptrace-via-utrace" + cleanups in utrace core.
>
> 1-7 are already in -mm tree, I am sending them to simplify the
> review.
>
> 8-12 don not change the behaviour, simple preparations.
>
> 13-14 add utrace-ptrace and utrace

Oleg,

I ran the ptrace-tests testsuite [1] on powerpc on the vanilla ptrace
and then with ptrace/utrace. The results for ptrace/utrace look better
:-)

All tests are 'make check xcheck'.

Ananth

[1] cvs�-d�:pserver:anoncvs:anoncvs(a)sources.redhat.com:/cvs/systemtap�co�ptrace-tests

---------
Vanilla ptrace:

PASS: ptrace-on-job-control-stopped
PASS: attach-wait-on-stopped
PASS: detach-can-signal
PASS: attach-into-signal
PASS: attach-sigcont-wait
PASS: sa-resethand-on-cont-signal
PASS: ptrace_cont-defeats-sigblock
PASS: ptrace-cont-sigstop-detach
PASS: ptrace_event_clone
PASS: tif-syscall-trace-after-detach
PASS: event-exit-proc-maps
PASS: event-exit-proc-environ
SKIP: x86_64-ia32-gs
SKIP: x86_64-gsbase
PASS: powerpc-altivec
PASS: peekpokeusr
PASS: watchpoint
PASS: block-step
PASS: step-jump-cont
SKIP: step-jump-cont-strict
PASS: ppc-dabr-race
PASS: signal-loss
PASS: step-into-handler
SKIP: user-area-access
PASS: user-regs-peekpoke
PASS: erestartsys
SKIP: erestart-debugger
SKIP: step-to-breakpoint
errno 14 (Bad address)
syscall-reset: syscall-reset.c:95: main: Assertion `(*__errno_location ()) == 38' failed.
unexpected child status 67f
FAIL: syscall-reset
PASS: reparent-zombie
PASS: step-simple
SKIP: step-through-sigret
PASS: stop-attach-then-wait
FAIL: detach-stopped
PASS: detach-stopped-rhel5
PASS: clone-multi-ptrace
PASS: clone-ptrace
PASS: o_tracevfork
PASS: o_tracevforkdone
PASS: detach-parting-signal
PASS: detach-sigkill-race
PASS: waitpid-double-report
PASS: o_tracevfork-parent
PASS: stopped-detach-sleeping
FAIL: stopped-attach-transparency
SKIP: erestartsys-trap
SKIP: highmem-debugger
PASS: sigint-before-syscall-exit
SKIP: syscall-from-clone
step-from-clone: step-from-clone.c:195: main: Assertion `(status >> 8) == 5' failed.
step-from-clone: step-from-clone.c:119: handler_fail: Assertion `0' failed.
/bin/sh: line 5: 19825 Aborted ${dir}$tst
FAIL: step-from-clone
step-fork: step-fork.c:56: handler_fail: Assertion `0' failed.
/bin/sh: line 5: 19832 Aborted ${dir}$tst
FAIL: step-fork
========================================
5 of 41 tests failed
(10 tests were not run)
Please report to utrace-devel(a)redhat.com
========================================
make[3]: *** [check-TESTS] Error 1
make[3]: Leaving directory `/home/ananth/ptrace-tests/tests'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory `/home/ananth/ptrace-tests/tests'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/ananth/ptrace-tests/tests'
make: *** [check-recursive] Error 1

---------
ptrace over utrace:

PASS: ptrace-on-job-control-stopped
PASS: attach-wait-on-stopped
PASS: detach-can-signal
PASS: attach-into-signal
PASS: attach-sigcont-wait
PASS: sa-resethand-on-cont-signal
PASS: ptrace_cont-defeats-sigblock
PASS: ptrace-cont-sigstop-detach
PASS: ptrace_event_clone
PASS: tif-syscall-trace-after-detach
PASS: event-exit-proc-maps
PASS: event-exit-proc-environ
SKIP: x86_64-ia32-gs
SKIP: x86_64-gsbase
PASS: powerpc-altivec
PASS: peekpokeusr
PASS: watchpoint
PASS: block-step
PASS: step-jump-cont
SKIP: step-jump-cont-strict
PASS: ppc-dabr-race
PASS: signal-loss
PASS: step-into-handler
SKIP: user-area-access
PASS: user-regs-peekpoke
PASS: erestartsys
SKIP: erestart-debugger
SKIP: step-to-breakpoint
errno 14 (Bad address)
syscall-reset: syscall-reset.c:95: main: Assertion `(*__errno_location ()) == 38' failed.
unexpected child status 67f
FAIL: syscall-reset
PASS: reparent-zombie
PASS: step-simple
SKIP: step-through-sigret
PASS: stop-attach-then-wait
PASS: detach-stopped
PASS: detach-stopped-rhel5
PASS: clone-multi-ptrace
PASS: clone-ptrace
PASS: o_tracevfork
PASS: o_tracevforkdone
PASS: detach-parting-signal
PASS: detach-sigkill-race
PASS: waitpid-double-report
PASS: o_tracevfork-parent
PASS: stopped-detach-sleeping
PASS: stopped-attach-transparency
SKIP: erestartsys-trap
SKIP: highmem-debugger
PASS: sigint-before-syscall-exit
SKIP: syscall-from-clone
SKIP: step-from-clone
step-fork: step-fork.c:56: handler_fail: Assertion `0' failed.
/bin/sh: line 5: 24803 Aborted ${dir}$tst
FAIL: step-fork
========================================
2 of 40 tests failed
(11 tests were not run)
Please report to utrace-devel(a)redhat.com
========================================
make[3]: *** [check-TESTS] Error 1
make[3]: Leaving directory `/home/ananth/ptrace-tests/tests'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory `/home/ananth/ptrace-tests/tests'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/ananth/ptrace-tests/tests'
make: *** [check-recursive] Error 1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Oleg Nesterov on
On 11/25, Ananth N Mavinakayanahalli wrote:
>
> I ran the ptrace-tests testsuite [1] on powerpc on the vanilla ptrace
> and then with ptrace/utrace. The results for ptrace/utrace look better
> :-)

Great! thanks a lot Ananth for doing this.

ptrace-utrace still fails 2 tests,

> FAIL: syscall-reset

I'll take a look later. Since unpatched kernel fails this test too
I am not going to worry right now. I think this is ppc specific, x86
passes this test.

> step-fork: step-fork.c:56: handler_fail: Assertion `0' failed.
> /bin/sh: line 5: 24803 Aborted ${dir}$tst
> FAIL: step-fork

This is expected. Should be fixed by

ptrace-copy_process-should-disable-stepping.patch

in -mm tree. (I am attaching this patch below just in case)
I din't mention this patch in this series because this bug
is "ortogonal" to utrace/ptrace.

Oleg.

------------------------------------------------------
If the tracee calls fork() after PTRACE_SINGLESTEP, the forked child
starts with TIF_SINGLESTEP/X86_EFLAGS_TF bits copied from ptraced parent.
This is not right, especially when the new child is not auto-attaced: in
this case it is killed by SIGTRAP.

Change copy_process() to call user_disable_single_step(). Tested on x86.

Test-case:

#include <stdio.h>
#include <unistd.h>
#include <signal.h>
#include <sys/ptrace.h>
#include <sys/wait.h>
#include <assert.h>

int main(void)
{
int pid, status;

if (!(pid = fork())) {
assert(ptrace(PTRACE_TRACEME) == 0);
kill(getpid(), SIGSTOP);

if (!fork()) {
/* kernel bug: this child will be killed by SIGTRAP */
printf("Hello world\n");
return 43;
}

wait(&status);
return WEXITSTATUS(status);
}

for (;;) {
assert(pid == wait(&status));
if (WIFEXITED(status))
break;
assert(ptrace(PTRACE_SINGLESTEP, pid, 0,0) == 0);
}

assert(WEXITSTATUS(status) == 43);
return 0;
}

Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
Acked-by: Roland McGrath <roland(a)redhat.com>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---


diff -puN kernel/fork.c~ptrace-copy_process-should-disable-stepping kernel/fork.c
--- a/kernel/fork.c~ptrace-copy_process-should-disable-stepping
+++ a/kernel/fork.c
@@ -1203,9 +1203,10 @@ static struct task_struct *copy_process(
p->sas_ss_sp = p->sas_ss_size = 0;

/*
- * Syscall tracing should be turned off in the child regardless
- * of CLONE_PTRACE.
+ * Syscall tracing and stepping should be turned off in the
+ * child regardless of CLONE_PTRACE.
*/
+ user_disable_single_step(p);
clear_tsk_thread_flag(p, TIF_SYSCALL_TRACE);
#ifdef TIF_SYSCALL_EMU
clear_tsk_thread_flag(p, TIF_SYSCALL_EMU);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on
On Tue, Nov 24, 2009 at 09:01:27PM +0100, Oleg Nesterov wrote:
> Hello.
>
> This is the new iteration of Roland's utrace patch, this time
> with "rewrite-ptrace-via-utrace" + cleanups in utrace core.
>
> 1-7 are already in -mm tree, I am sending them to simplify the
> review.
>
> 8-12 don not change the behaviour, simple preparations.
>
> 13-14 add utrace-ptrace and utrace

Skipped over it very, very briefly. One thing I really hate about this
is that it introduces two ptrace implementation by adding the new one
without removing the old one. Given that's it's pretty much too later
for the 2.6.33 cycle anyway I'd suggest you make sure the remaining
two major architectures (arm and mips) get converted, and if the
remaining minor architectures don't manage to get their homework done
they're left without ptrace.

The other thing is that this patchset really doesn't quite justify
utrace. It's growing a lot more code without actually growing any
useful functionality. What about all those other utrace killer
features that have been promised for a long time?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Oleg Nesterov on
On 11/25, Christoph Hellwig wrote:
>
> On Tue, Nov 24, 2009 at 09:01:27PM +0100, Oleg Nesterov wrote:
> > Hello.
> >
> > This is the new iteration of Roland's utrace patch, this time
> > with "rewrite-ptrace-via-utrace" + cleanups in utrace core.
> >
> > 1-7 are already in -mm tree, I am sending them to simplify the
> > review.
> >
> > 8-12 don not change the behaviour, simple preparations.
> >
> > 13-14 add utrace-ptrace and utrace
>
> Skipped over it very, very briefly. One thing I really hate about this
> is that it introduces two ptrace implementation by adding the new one
> without removing the old one.

Yes, we obviously need the old one when CONFIG_UTRACE is not enabled.

So, I'd like to try to restate: one thing we all really hate is that
CONFIG_UTRACE exists.

> Given that's it's pretty much too later
> for the 2.6.33 cycle anyway I'd suggest you make sure the remaining
> two major architectures (arm and mips) get converted, and if the
> remaining minor architectures don't manage to get their homework done
> they're left without ptrace.

Well, I can't comment this. I mean, I can't judge.

> The other thing is that this patchset really doesn't quite justify
> utrace. It's growing a lot more code without actually growing any
> useful functionality.

This should be clarified. I don't think ptrace-utrace adds a lot more
code compared to the old ptrace. Note that we can kill a lot of old
code once CONFIG_UTRACE goes away. ptrace_signal(), ptrace_notify(),
even task_struct->almost_all_ptrace_related can go away.

kernel/utrace.c does add 12280 bytes (on my machine), yes.

> What about all those other utrace killer
> features that have been promised for a long time?

It is not clear how we can expect the new "killer" modules/applications
which use utrace before we merge it.

We already have some users, say, systemtap. But I don not know
what can be counted as a "really killer" application of utrace.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Srikar Dronamraju on
Hi Christoph,

>
> The other thing is that this patchset really doesn't quite justify
> utrace. It's growing a lot more code without actually growing any
> useful functionality. What about all those other utrace killer
> features that have been promised for a long time?
>

We are working on in-kernel gdbstub which was one of the features that
you had asked for. gdbstub does pass unit tests; but we are looking at
some way to hack the GDB testsuite to run its regression tests. Once we
are able to run the GDB testsuite and utrace is part of some upstream
tree, we plan to post these patches to LKML for comments. gdbstub uses
utrace and uprobes underneath. Uprobes was rewritten to remove issues
that LKML developers had opposed. Uprobes also has its own ftrace plugin
to use uprobes.

Currently in-kernel gdbstub is hosted by Frank Ch. Eigler over here:
git://web.elastic.org/~fche/utrace-ext.git
branch name utrace-gdbstub-uprobes

--
Regards
Srikar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/