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

[Daniel Jacobowitz <dan@debian.org>]

I'll reply to the rest of this in a moment, but one thing at a time...

On Fri, Sep 06, 2002 at 02:08:13AM +0900, OGAWA Hirofumi wrote:
> Ingo Molnar <mingo@elte.hu> writes:
> 
> > Linus,
> > 
> > the attached patch (against BK-curr) collects two ptrace related fixes:  
> > first it undoes Ogawa's change (so various uses of ptrace works again),
> > plus it adds Daniel's suggested fix that allows a parent to PTRACE_ATTACH
> > to a child it forked. (this also fixes the incorrect BUG_ON() assert
> > Ogawa's patch was intended to fix in the first place.)
> > 
> > i've tested various ptrace uses and they appear to work just fine.
> > 
> > (Daniel, let us know if you can still see anything questionable in this
> > area - or if the ptrace list could be managed in a cleaner way.)


> in sys_wait4()
> 
> +				} else {
> +					if (p->ptrace) {
> +						write_lock_irq(&tasklist_lock);
> +						ptrace_unlink(p);
> +						write_unlock_irq(&tasklist_lock);
> +					}
>  					release_task(p);
> +				}
> 
> Umm, why needed this? If ->real_parent == ->parent, it's real
> child. So this child don't use ->ptrace_list.

You're right, we just need to clear p->ptrace.  And there was a problem
with debugged detached tasks.  Ingo, does this look right to you?  It
passes my testing.  Handle unlinking in release_task instead of at both
call sites, since they both need it.

===== exit.c 1.45 vs edited =====
*** /tmp/exit.c-1.45-26998	Mon Sep  2 01:15:09 2002
--- exit.c	Thu Sep  5 14:23:32 2002
*************** static void release_task(struct task_str
*** 66,71 ****
--- 66,76 ----
  	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));
  	unhash_process(p);
  
===== ptrace.c 1.16 vs edited =====
*** /tmp/ptrace.c-1.16-26998	Mon Aug 19 14:12:27 2002
--- ptrace.c	Thu Sep  5 14:18:05 2002
*************** void __ptrace_link(task_t *child, task_t
*** 29,35 ****
  	if (!list_empty(&child->ptrace_list))
  		BUG();
  	if (child->parent == new_parent)
! 		BUG();
  	list_add(&child->ptrace_list, &child->parent->ptrace_children);
  	REMOVE_LINKS(child);
  	child->parent = new_parent;
--- 29,35 ----
  	if (!list_empty(&child->ptrace_list))
  		BUG();
  	if (child->parent == new_parent)
! 		return;
  	list_add(&child->ptrace_list, &child->parent->ptrace_children);
  	REMOVE_LINKS(child);
  	child->parent = new_parent;


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer