get_utrace_lock() checks -state != EXIT_DEAD to make sure it safe
to use -utrace. This is unneeded since -utrace was embedded into
task_struct. If we can read -state, we can read -utrace as well.
Signed-off-by: Oleg Nesterov o...@redhat.com
kernel/utrace.c | 26
/SIGNAL_STOP_STOPPED.
This allows to greatly simplify utrace_report_jctl() and avoid playing
with group-stop bookkeeping twice.
Signed-off-by: Oleg Nesterov o...@redhat.com
kernel/signal.c | 40 ++--
kernel/utrace.c | 20
2 files changed, 18
Introduce the empty inline tracehook_finish_jctl() helper called by
do_signal_stop() after wakeup.
Currently we lack the ability to report this state change.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
include/linux/tracehook.h |9 +
kernel/signal.c |2 ++
2
-by: Oleg Nesterov o...@redhat.com
---
kernel/utrace.c | 36 +---
1 file changed, 1 insertion(+), 35 deletions(-)
--- __UTRACE/kernel/utrace.c~JCTL_3_CLEANUP 2009-07-28 23:41:54.0
+0200
+++ __UTRACE/kernel/utrace.c2009-07-28 23:59:28.0
utrace_do_stop() sets utrace-stopped but leaves the tracee in TASK_STOPPED
state. This means SIGCONT can wake up the tracee and fool the tracer.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/utrace.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- __UTRACE/kernel
On 07/28, Roland McGrath wrote:
+* While in TASK_STOPPED, we can be considered safely
+* stopped by utrace_do_stop(). Clear -stopped if we
+* were woken by signal.
s/signal/SIGKILL/, no? It's not really while in TASK_STOPPED any more,
it's that utrace_do_stop could have
On 07/28, Roland McGrath wrote:
utrace_do_stop() sets utrace-stopped but leaves the tracee in TASK_STOPPED
state. This means SIGCONT can wake up the tracee and fool the tracer.
IMHO this one belongs before 2/4 and 3/4 and all the comments changed to
reflect that SIGKILL is the only case.
On 07/28, Roland McGrath wrote:
Questions: should I send the change in signal.c to Andrew right now?
It can be applied separetely, it doesn't break utrace_report_jctl().
Yes, send it now.
OK, will do
It might be best just to send a replacement
for
On 07/28, Roland McGrath wrote:
OK, I'll send the first patch upstream, then I re-send 2-4 with updated
comments to you.
Ok, good. When I get those I'll merge those upstream-bound ones into the
tracehook git branch (now just signals-tracehook_notify_jctl-change.patch)
and merge the
Damn. Please ignore.
Somehow I missed the fact that the patch below changes both tracehooks
and utrace.c, it is not possible to send the tracehook part to Andrew.
But I think it would be really nice to do this cleanup before sending
the next version of utrace.
Oleg.
On 07/29, Oleg Nesterov
Andrew, this is on top of
signals-tracehook_notify_jctl-change.patch
and in fact it can be folded into that patch.
Or I can send the unified patch as a replacement, whatever is more
convenient to you.
But, to clarify, the patch above is correct, this one just tries to
improve things,
/SIGNAL_STOP_STOPPED is set only when we
know for sure we are going to schedule() after unlock(siglock).
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/signal.c | 40 ++--
1 file changed, 18 insertions(+), 22 deletions(-)
--- __UTRACE/kernel/signal.c
-off-by: Oleg Nesterov o...@redhat.com
---
include/linux/tracehook.h |9 +
kernel/signal.c |2 ++
2 files changed, 11 insertions(+)
--- __UTRACE/include/linux/tracehook.h~2_FINISH_JCTL2009-07-29
03:10:10.0 +0200
+++ __UTRACE/include/linux/tracehook.h 2009-07
On 07/24, Roland McGrath wrote:
Roland, can we just remove all code which plays with -stopped in
utrace_report_jctl() and utrace_get_signal(), and do something like
[...]
@@ -1602,6 +1602,16 @@ static int do_signal_stop(int signr)
do {
schedule();
utrace_wakeup:
if (likely(task_is_stopped_or_traced(target))) {
if (target-signal-flags SIGNAL_STOP_STOPPED)
target-state = TASK_STOPPED;
else
wake_up_state(target, __TASK_STOPPED | __TASK_TRACED);
}
On 07/27, Oleg Nesterov wrote:
On 07/24, Roland McGrath wrote:
I recall you wanted to rework something about tracehook_notify_jctl earlier
too. Now is the time to revisit all that and clean the code up however
seems best. It shouldn't be a problem to rewrite the last half
On 07/27, Roland McGrath wrote:
No, it can't. That's what get the bookkeeping right means.
It the debugger uses UTRACE_RESUME et al, then that thread moves
from TASK_TRACED into TASK_STOPPED and still never runs.
Ah. In that case yes, we can make this all consistent.
But I am not sure I
On 07/27, Roland McGrath wrote:
utrace_wakeup:
if (likely(task_is_stopped_or_traced(target))) {
if (target-signal-flags SIGNAL_STOP_STOPPED)
target-state = TASK_STOPPED;
else
wake_up_state(target, __TASK_STOPPED |
On 07/27, Roland McGrath wrote:
Assuming you agree with this change... I don't know how it should be merged.
Probably the change in signal.c should be sent separately, but this breaks
-mm tree.
The relevant -mm differences are just in one patch that folds finish_stop
into do_signal_stop,
On 07/22, Roland McGrath wrote:
serves for attach/detach. You need somewhere to store the
PTRACE_SETOPTIONS state and so forth, sure. But you can probably just
handle the attachedness at the utrace level. That's what
UTRACE_ATTACH_EXCLUSIVE is for.
Yes. As for -ptrace, I think
On 07/22, Roland McGrath wrote:
This means ptrace_check_attach() can succeed but the tracee can even return
to the user-space after that, no? There is no guarantee utrace_get_signal()
will call finish_resume_report(). For example, if there are no pending
signals, we just clear -stopped
On 07/22, Roland McGrath wrote:
I still think EXIT_DEAD check must die ;)
In get_utrace_lock, you mean? Do you mean it's superfluous because we can
rely on utrace_reap having cleared engine-ops before it matters to us?
Yes. Perhaps I missed something, but this check buys nothing. First of
On 07/21, Roland McGrath wrote:
For example, utrace_control(UTRACE_STOP)-utrace_do_stop() finds
the child inside utrace_report_jctl() path in TASK_STOPPED state,
Where? utrace_report_jctl is called with the siglock held, and resets
to TASK_RUNNING before it unlocks.
sets utrace-stopped
On 07/16, Roland McGrath wrote:
Looks like a horrible hack, imho. Utrace should know nothing about
ptrace.
Agreed!
Can't we do something like
[...]
+ // SPECIAL!!! must not block/etc
+ void (*notify_stopped)(...);
Part of the point about utrace is to make it
First of all, I believe ptrace_check_attach() is buggy. It lacks
child-parent == current or engine-data == current check.
The bug is serious, sys_ptrace() does not do security checks unless
request == PTRACE_ATTACH. But this is trivial to fix.
Also. Without utrace, ptrace_check_attach() requires
I fail to understand rcu_read_lock() + get_task_struct() in
utrace_prepare_examine(). This looks as if the caller does not need to
make sure task_struct can't go away.
But, unless the caller does get_task_struct() itself (like ptrace does),
utrace_prepare_examine() can race with utrace_reap() (if
Shouldn't be applied without Roland's ack.
I just don't know how to merge the second patch properly. I think it would be
really nice to cleanup ptrace attach before ptracee data structures cleanup,
but this depends on utrace-core.patch which adds exclude_ptrace().
With the first patch, the
we need is to make sure we can't attach after exit_notify(), check
task-exit_state.
And finally, move ptrace_traceme() up near ptrace_attach() to keep them
close to each other.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/ptrace.c | 127
On 04/20, Roland McGrath wrote:
Afaics ptrace_attach() needs this lock only to pin -mm, no other other
reasons. ptrace_traceme() doesn't need it at all.
I'm pretty sure that -mm check is only meant to exclude kernel threads.
It should check PF_KTHREAD now,
Yes.
But
On 04/19, Roland McGrath wrote:
X-real_parent calls do_wait() and gets -ECHLD, because X is EXIT_DEAD
and not traced.
I see. I don't think we should worry especially about fixing this existing
bug (ancient, as you call it) on its own. So let's only consider this in
the context of the
On 04/16, Roland McGrath wrote:
But even that is a lot of hair for the incremental patches in the first
several stages, I think. So just never deallocate it, and:
static inline int task_ptrace(struct task_struct *task)
{
return unlikely(task-ptrace_child) ?
On 04/16, Roland McGrath wrote:
OK. For the moment, please forget about these changes. Let's recall
do_wait() has the ancient bug. wait_task_zombie() sets EXIT_DEAD
unconditionally, then drops tasklist. If we are not the real_parent
and the child was traced, we may restore exit_state =
On 04/13, Roland McGrath wrote:
I tried to think about the first steps in ptrace-cleanup, and I
need your help.
I think I said that list was not necessarily in this order and I
certainly meant it so.
Yes sure. But I think this part is most important and most hard, and
other changes may
(add utrace-devel)
On 03/30, Roland McGrath wrote:
* ptracer (parent) data structures cleanup
** move ptraced list into ptracer sub-struct
** add mutex locking ptracer
*** locks ptraced list, not tasklist_lock
*** locks ptrace_do_wait vs SIGCHLD/wakeup
*** locks tracees' ptrace flags, not
On 03/25, Roland McGrath wrote:
I also really don't understand the resistance to a new thing in a
new config option that depends on EXPERIMENTAL, and having the
smaller users bang on it and fix it in the tree for a while.
And, just in case...
Without CONFIG_UTRACE, the patch does not change
bookkeeping twice.
Signed-off-by: Oleg Nesterov o...@redhat.com
signal.c | 29 +++--
utrace.c | 20
2 files changed, 11 insertions(+), 38 deletions(-)
--- xxx/kernel/signal.c~JCTL_SIMPLIFY 2009-03-18 14:50:06.0 +0100
+++ xxx/kernel/signal.c
On 03/18, Roland McGrath wrote:
Yes, but the other side lacks a barrier. UTRACE_REPORT does
utrace-report = 1;
wmb(); // actually mb, but wmb is enough
set _TIF_NOTIFY_RESUME;
do_notify_resume()-utrace_resume()-start_report() path does
if (_TIF_NOTIFY_RESUME)
On 03/18, Oleg Nesterov wrote:
On 03/18, Roland McGrath wrote:
It's OK to change the tracehook definition. I did this on the new git
branch tracehook, then utrace branch commit 7b0be6e4 merges that and uses
it.
Roland, I think it better to change tracehook definition more, please
On 03/15, Roland McGrath wrote:
Then we re-do this (well, almost) check under -siglock,
} else if (task_is_stopped(target)) {
if (!(target-utrace_flags UTRACE_EVENT(JCTL)))
utrace-stopped = stopped = true;
}
But this is not nice. Let's
On 03/15, Roland McGrath wrote:
utrace_set_events:
(utrace-death ((old_flags ~events) DEATH_EVENTS))
(old_flags ~events) DEATH_EVENTS) means the caller tries to
clear DEATH/QUIESCE. Why this is not allowed? And why this is not
allowed _only_ when the target runs
On 03/16, Mathieu Desnoyers wrote:
utrace_add_engine()
set_notify_resume(target);
ok, so this is where the TIF_NOTIFY_RESUME thread flag is set. I notice
that it is set asynchronously with the execution of the target thread
(as I do with my TIF_KERNEL_TRACE thread flag).
However, on
On 03/12, Roland McGrath wrote:
So I think we need this:
@@ -181,7 +181,13 @@ static int utrace_add_engine(struct task_struct *target,
* also set. Otherwise utrace_control() or utrace_do_stop()
* might skip setting TIF_NOTIFY_RESUME upon seeing -report
utrace_set_events:
(utrace-death ((old_flags ~events) DEATH_EVENTS))
(old_flags ~events) DEATH_EVENTS) means the caller tries to
clear DEATH/QUIESCE. Why this is not allowed? And why this is not
allowed _only_ when the target runs utrace_report_death()-REPORT()?
I think this line
On 03/12, Roland McGrath wrote:
Yep. And utrace_reset() can be called because -stopped == 1.
Right.
Let me explain. Again, let's suppose D attaches engine E to the target T.
T enters utrace_report_jctl() with -stopped == 1.
D calls utrace_set_events(events = 0), this removes JCTL
On 03/12, Renzo Davoli wrote:
I have update also the second patch. Please note that now this patch
must be applied after the first one.
This patch implements a consistent nesting model for utrace machines.
(There is a full description in the messages I sent on Feb. 14 and Mar. 6)
This patch
Roland, I left some parts of your message unanswered because I need to think
more about them...
On 03/12, Roland McGrath wrote:
But, if we enter utrace_report_jctl() with -stopped == 1, JCTL can be
already removed from -utrace_flags, exactly because -stopped was true.
I don't follow this.
I'm afraid I wasn't clear again,
On 03/12, Oleg Nesterov wrote:
Oh. utrace_attach_task()-utrace_add_engine() sets -report +
TIF_NOTIFY_RESUME.
But tracehook_notify_resume() does nothing because -utrace_flags == 0 ?
Confused.
Perhaps this is not problem per se. But let's suppose we call
On 03/11, Roland McGrath wrote:
But not vise versa. I misunderstood the comment as if the new engine
should not be notified if it is attached by another task while target
is inside callback.
That is indeed what happens in that case. But that one is not a
specific should not, it's just
On 03/03, Roland McGrath wrote:
I would rather not touch the tracehook interfaces now. You are indeed
right that the motivation for this had to do with the utrace-indirect code.
As I've said, I do intend to resurrect that code and send it upstream later
on. We can consider cleanups then.
this argument to
utrace_report_death().
Looks like this is not needed any longer, kill this awful cookie.
Signed-off-by: Oleg Nesterov o...@redhat.com
--- xxx/include/linux/utrace.h~KILL_COOKIE 2009-03-03 18:11:47.0
+0100
+++ xxx/include/linux/utrace.h 2009-03-03 20:43:43.0 +0100
misleading, kill this check.
Also remove the comment, the comment above get_utrace_lock() explains
that the caller has to hold a ref on the engine.
Signed-off-by: Oleg Nesterov o...@redhat.com
--- xxx/kernel/utrace.c~WRONG_REFCNT_CK 2009-03-03 20:46:09.0 +0100
+++ xxx/kernel/utrace.c 2009-03-03
501 - 551 of 551 matches
Mail list logo
