Messages in this thread Patch in this message | | | Date | Mon, 31 Mar 2008 13:12:15 +0400 | From | Oleg Nesterov <> | Subject | Re: [PATCH 2/3] ptrace children revamp |
| |
On 03/30, Roland McGrath wrote: > > +static void ptrace_exit(struct task_struct *parent, struct list_head *dead) > { > - if (p->pdeath_signal) > - /* We already hold the tasklist_lock here. */ > - group_send_sig_info(p->pdeath_signal, SEND_SIG_NOINFO, p); > + struct task_struct *p, *n; > > - /* Move the child from its dying parent to the new one. */ > - if (unlikely(traced)) { > - /* Preserve ptrace links if someone else is tracing this child. */ > - list_del_init(&p->ptrace_list); > - if (p->parent != p->real_parent) > - list_add(&p->ptrace_list, &p->real_parent->ptrace_children); > - } else { > - /* If this child is being traced, then we're the one tracing it > - * anyway, so let go of it. > - */ > - p->ptrace = 0; > - remove_parent(p); > - p->parent = p->real_parent; > - add_parent(p); > + list_for_each_entry_safe(p, n, &parent->ptrace_attach, ptrace_list) { > + __ptrace_unlink(p); > > - if (task_is_traced(p)) { > - /* > - * If it was at a trace stop, turn it into > - * a normal stop since it's no longer being > - * traced. > - */ > - ptrace_untrace(p); > + /* > + * If it's a zombie, our attachedness prevented normal > + * parent notification or self-reaping. Do notification > + * now if it would have happened earlier. If it should > + * reap itself, add it to the @dead list. We can't call > + * release_task() here because we already hold tasklist_lock. > + */ > + if (p->exit_state == EXIT_ZOMBIE) { > + if (p->exit_signal != -1 && !thread_group_empty(p)) ^^^^^^^^^^^^^^^^^^^^^^^ I still think this is wrong. Please also look at my previous reply, http://marc.info/?l=linux-kernel&m=120680153931177
> @@ -1481,6 +1478,15 @@ static int wait_consider_task(struct task_struct *parent, > */ > *retval = 0; > > + if (unlikely(p->parent != parent)) { > + /* > + * This child is hidden by someone else who did PTRACE_ATTACH. > + * We aren't allowed to see it now, but eventually we will. > + */ > + *retval = 0; > + return 0; > + }
Looks wrong... Above this "p->parent != parent" check we have
if (p->exit_state == EXIT_ZOMBIE && !delay_group_leader(p)) return wait_task_zombie(p, options, infop, stat_addr, ru);
Suppose that p is not a group leader, it is traced by some unrelated task, and now it is EXIT_ZOMBIE. We shouldn't release it (and return the "bogus" pid to user-space).
Oleg.
| |