Messages in this thread | | | Date | Wed, 27 Aug 2008 20:36:19 +0400 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/4] pid_ns: zap_pid_ns_processes: fix the ->child_reaper changing |
| |
On 08/26, Serge E. Hallyn wrote: > > Quoting Oleg Nesterov (oleg@tv-sign.ru): > > zap_pid_ns_processes() sets pid_ns->child_reaper = NULL, this is wrong. > > ... > > Even if there are no childs, it is not good that forget_original_parent() > > uses reaper == NULL. > > ... > > Well while it looked correct to me all along, I couldn't get the > testcase to cause an oops.
Perhaps you can start several instances or increase the delay, I forgot that usleep() uses usecs, not msecs. In any case I agree, the test-case is oversimplified, but since it worked for me I didn't try to make it more "stubborn".
> Acked-by: Serge Hallyn <serue@us.ibm.com>
Thanks!
> > + /* > > + * We can not clear ->child_reaper or leave it alone. > > + * There may by stealth EXIT_DEAD tasks on ->children, > > + * forget_original_parent() must move them somewhere. > > Actually this comment is a little bit misleading - the null > deref will happen regardless of whether there are children, > right?
Yes, the comment could be better ;)
But no, we won't oops without children. Please note that forget_original_parent() uses reaper inside the list_for_each_entry_safe(father->children) loop. ->children is empty, reaper is not used at all.
But please also note the "not good" part of the changelog above.
Thanks to all for review!
Oleg.
| |