Re: [PATCH 07/16] signal: Wake up the designated parent

Oleg Nesterov
2022-05-24T13:25:54Z

I fail to understand this patch...

On 05/18, Eric W. Biederman wrote:
>
> Today if a process is ptraced only the ptracer will ever be woken up in
> wait

and why is this wrong?

> Fixes: 75b95953a569 ("job control: Add @for_ptrace to do_notify_parent_cldstop()")

how does this change fix 75b95953a569?

> static int child_wait_callback(wait_queue_entry_t *wait, unsigned mode,
> int sync, void *key)
> {
> struct wait_opts *wo = container_of(wait, struct wait_opts,
> child_wait);
> - struct task_struct *p = key;
> + struct child_wait_info *info = key;
>
> - if (!eligible_pid(wo, p))
> + if (!eligible_pid(wo, info->p))
> return 0;
>
> - if ((wo->wo_flags & __WNOTHREAD) && wait->private != p->parent)
> - return 0;
> + if ((wo->wo_flags & __WNOTHREAD) && (wait->private != info->parent))
> + return 0;

So. wait->private is the task T which sleeping on wait_chldexit.

Before the patch the logic is clear. T called do_wait(__WNOTHREAD) and
we do not need to wake it up if it is not the "actual" parent of p.

After the patch we check it T is actual to the "parent" arg passed to
__wake_up_parent(). Why??? This arg is only used to find the
->signal->wait_chldexit wait_queue_head, and this is fine.

As I said, I don't understand this patch. But at least this change is
wrong in case when __wake_up_parent() is calles by __ptrace_detach().
(you removed it in 5/16 but this looks wrong too). Sure, we can change
ptrace_detach() to use __wake_up_parent(p, p->parent), but for what?

I must have missed something.

Oleg.