[PATCH] utrace: utrace_get_signal() must check -pending_attach
On 11/15, Oleg Nesterov wrote:
On 11/13, Oleg Nesterov wrote:
Roland, I pulled the last changes in your tree (utrace-cleanup merged
in utrace-ptrace) and did some testing.
In short: utrace-ptrace does not work at all.
It fails a lot (if not most) of tests, in particular
sa-resethand-on-cont-signal hangs and clone-multi-ptrace silently
crashes the kernel.
The quick investigation didn't help, utrace.c has too much changes,
I will read the code tomorrow from the very beginning.
Oh.
In short, the code in utrace.c was broken even before utrace-cleanup
was merged
And your current tree has the same problem: utrace_attach_task() sets
-pending_attach, but utrace_get_signal() can miss it and dequeue/report
a signal without splice_attaching().
Change utrace_get_signal() to check -pending_attach.
I do not understand the new code yet, I am not sure this patch is optimal.
But it helps, the kernel doesn't crash/hang any longer.
We still have problems, 5 tests fail. Some failures are expected, we know
that the changes in utrace-cleanup must break the stepping logic in ptrace
(to remind you just in case, utrace_control(STEP) doesn't call enable_step()
synchronously). Some are not, ptrace_event_clone for example.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/utrace.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- UTRACE-PTRACE/kernel/utrace.c~UTRACE_ATTACH_SIGNAL_RACE 2009-11-13
18:00:06.0 +0100
+++ UTRACE-PTRACE/kernel/utrace.c 2009-11-15 19:52:29.0 +0100
@@ -1955,7 +1955,8 @@ int utrace_get_signal(struct task_struct
int signr;
utrace = task_utrace_struct(task);
- if (utrace-resume UTRACE_RESUME || utrace-signal_handler) {
+ if (utrace-resume UTRACE_RESUME ||
+ utrace-pending_attach || utrace-signal_handler) {
enum utrace_resume_action resume;
/*
Re: [PATCH] utrace: finish_report() must never set -resume = UTRACE_STOP
On 11/15, Oleg Nesterov wrote: With this patch the kernel passes all tests except single-step ones, as expected. Forgot to mention, your tree lacks these patches we sent upstream: ptrace-introduce-user_single_step_siginfo-helper.patch ptrace-powerpc-implement-user_single_step_siginfo.patch ptrace-change-tracehook_report_syscall_exit-to-handle-stepping.patch ptrace-x86-implement-user_single_step_siginfo.patch ptrace-x86-change-syscall_trace_leave-to-rely-on-tracehook-when-stepping.patch I applied them to my tree, otherwise ptrace.c can't be compiled. Oleg.
[PATCH] utrace: utrace_attach_task() forgets to return when -utrace == NULL
I forgot about make xcheck, it crashes the kernel. Fortunately the
kernel dumps the stack trace. Trust me, it wasn't easy to notice the
missing return ;) I am wondering why the compiler doesn't complain.
Roland, this all needs more fixes. Look at the fixed code,
utrace = target-utrace;
if (!utrace)
return ERR;
spin_lock(utrace-lock);
Now, if we race with another task doing utrace_task_alloc() and see
-utrace != NULL, why should we see the correctly initialized *utrace?
utrace_task_alloc() needs wmb(), and the code like above read_barrier_depends().
UPD: tested the kernel with this patch, now late-ptrace-may-attach-check
crashes the kernel silently (no output under kvm).
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/utrace.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- UTRACE-PTRACE/kernel/utrace.c~UTRACE_ATTACH_FIX_UTRACE_CK 2009-11-16
00:02:08.0 +0100
+++ UTRACE-PTRACE/kernel/utrace.c 2009-11-16 00:06:26.0 +0100
@@ -281,7 +281,7 @@ struct utrace_engine *utrace_attach_task
if (!(flags UTRACE_ATTACH_CREATE)) {
if (unlikely(!utrace))
- ERR_PTR(-ENOENT);
+ return ERR_PTR(-ENOENT);
spin_lock(utrace-lock);
engine = matching_engine(utrace, flags, ops, data);
if (engine)
Re: [PATCH] utrace: utrace_attach_task() forgets to return when -utrace == NULL
On 11/16, Oleg Nesterov wrote: UPD: tested the kernel with this patch, now late-ptrace-may-attach-check crashes the kernel silently (no output under kvm). Repeated this test. Got several oopses, bad spinlock magic in utrace-lock. The stack trace varies, often from utrace_get_signal(). Oh, will continue tomorrow. Oleg.
