On 12/04, Roland McGrath wrote:
>
> > I think the problem is clear now.
>
> Ok. We should probably move this discussion to utrace-devel.
Yes, I didn't notice we discuss this offlist...
> > I forgot that there is another issue (iirc a bit discussed too).
> > finish_callback_report() sets ->ops = utrace_detached_ops lockless!
>
> You'll have to remind me why this is a problem.
Re: [PATCH 85] ptrace_attach_task: rely on utrace_barrier(), don't
check ->ops
https://www.redhat.com/archives/utrace-devel/2009-October/msg00180.html
We already discussed this, but forgot to finish.
Do you agree with the patch?
------------------------------------------------------------------------------
[PATCH] utrace: don't set ->ops = utrace_detached_ops lockless
finish_callback_report() changes ->ops lockless. Imho this is not
right "in general", the state of !EXIT_DEAD tracee must be stable
under utrace->lock.
And this can confuse ptrace_reuse_engine()->utrace_barrier() logic.
utrace_barrier() can race with reporting loop and return 0 while
engine was already detached or in the middle of detach.
See also https://www.redhat.com/archives/utrace-devel/2009-October/msg00180.html
Signed-off-by: Oleg Nesterov <o...@redhat.com>
---
kernel/utrace.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
--- UTRACE-PTRACE/kernel/utrace.c~DONT_CHANGE_OPS_LOCKLESS 2009-11-24
17:20:33.000000000 +0100
+++ UTRACE-PTRACE/kernel/utrace.c 2009-12-04 17:10:37.000000000 +0100
@@ -1390,11 +1390,15 @@ static inline void finish_callback_repor
struct utrace_engine *engine,
enum utrace_resume_action action)
{
+ if (action == UTRACE_DETACH) {
+ spin_lock(&utrace->lock);
+ engine->ops = &utrace_detached_ops;
+ spin_unlock(&utrace->lock);
+ }
/*
* If utrace_control() was used, treat that like UTRACE_DETACH here.
*/
- if (action == UTRACE_DETACH || engine->ops == &utrace_detached_ops) {
- engine->ops = &utrace_detached_ops;
+ if (engine->ops == &utrace_detached_ops) {
report->detaches = true;
return;
}
