Re: [patch] ptrace-fix-2.5.33-A1

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Ingo Molnar <mingo@elte.hu> writes:

> i've attached a combined patch of your two patches, against BK-curr. Looks
> good to me, and since it passed your more complex ptrace tests ...
> 
> 	Ingo
> 
> --- linux/kernel/exit.c.orig	Fri Sep  6 00:55:02 2002
> +++ linux/kernel/exit.c	Fri Sep  6 00:57:58 2002
> @@ -66,6 +66,11 @@
>  	atomic_dec(&p->user->processes);
>  	security_ops->task_free_security(p);
>  	free_uid(p->user);
> +	if (unlikely(p->ptrace)) {
> +		write_lock_irq(&tasklist_lock);
> +		__ptrace_unlink(p);
> +		write_unlock_irq(&tasklist_lock);
> +	}
>  	BUG_ON(!list_empty(&p->ptrace_list) || !list_empty(&p->ptrace_children));

Looks like it's need the only CLONE_DETACH process. Why it's here?

	 * Search them and reparent children.
	 */
	list_for_each(_p, &father->children) {
		p = list_entry(_p,struct task_struct,sibling);
		reparent_thread(p, reaper, child_reaper);
	}

Looks like that tracer deprive a process from real parent.

	list_for_each(_p, &father->ptrace_children) {
		p = list_entry(_p,struct task_struct,ptrace_list);
		reparent_thread(p, reaper, child_reaper);
	}

Thread group makes the child which links both ->children and
->ptrace_children.

>  {
> -	ptrace_unlink(p);
> -	list_del_init(&p->sibling);
> -	p->ptrace = 0;
> +	/* If we were tracing the thread, release it; otherwise preserve the
> +	   ptrace links.  */
> +	if (unlikely(traced)) {
> +		task_t *trace_task = p->parent;
> +		__ptrace_unlink(p);
> +		p->ptrace = 1;

Unexpected change of ptrace flag.

> +		__ptrace_link(p, trace_task);
> +	} else {
> +		p->ptrace = 0;
> +		list_del_init(&p->sibling);
> +		p->parent = p->real_parent;
> +		list_add_tail(&p->sibling, &p->parent->children);

Looks like that tracing child still link ->ptrace_list.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>