lkml.org 
[lkml]   [2008]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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.



\
 
 \ /
  Last update: 2008-03-31 12:15    [W:0.058 / U:0.532 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site