Messages in this thread | | | Date | Fri, 19 Jul 2019 18:53:46 +0200 | Subject | Re: [PATCH RFC v1] pidfd: fix a race in setting exit_state for pidfd polling | From | Christian Brauner <> |
| |
On July 19, 2019 6:51:20 PM GMT+02:00, Joel Fernandes <joelaf@google.com> wrote: >On Fri, Jul 19, 2019 at 12:27 PM Christian Brauner ><christian@brauner.io> wrote: >> >> On Fri, Jul 19, 2019 at 06:14:05PM +0200, Oleg Nesterov wrote: >> > it seems that I missed something else... >> > >> > On 07/17, Joel Fernandes (Google) wrote: >> > > >> > > @@ -1156,10 +1157,11 @@ static int wait_task_zombie(struct >wait_opts *wo, struct task_struct *p) >> > > ptrace_unlink(p); >> > > >> > > /* If parent wants a zombie, don't release it now */ >> > > - state = EXIT_ZOMBIE; >> > > + p->exit_state = EXIT_ZOMBIE; >> > > if (do_notify_parent(p, p->exit_signal)) >> > > - state = EXIT_DEAD; >> > > - p->exit_state = state; >> > > + p->exit_state = EXIT_DEAD; >> > > + >> > > + state = p->exit_state; >> > > write_unlock_irq(&tasklist_lock); >> > >> > why do you think we also need to change wait_task_zombie() ? >> > >> > pidfd_poll() only needs the exit_state != 0 check, we know that it >> > is not zero at this point. Why do we need to change exit_state >before >> > do_notify_parent() ? >> >> Oh, because of?: >> >> /* >> * Move the task's state to DEAD/TRACE, only one thread can >do this. >> */ >> state = (ptrace_reparented(p) && thread_group_leader(p)) ? >> EXIT_TRACE : EXIT_DEAD; >> if (cmpxchg(&p->exit_state, EXIT_ZOMBIE, state) != >EXIT_ZOMBIE) >> return 0; >> >> So exit_state will definitely be set in this scenario. Good point. >> > >Agreed. Christian, do you mind dropping this hunk from the patch or do >you want me to resend the patch with the hunk dropped?
Yeah, no problem. :)
| |