I still think we can avoid the new lock and rely on RCU in
kill_something_info() and kill_pgrp().
I am attaching my old email below. It talks about pgrp, however I
think kill_something_info() is almost the same thing.
Oleg.
On 12/07, Eric W. Biederman wrote:
Yes, but who can add the new entry?
Let's forget about setpgrp/etc for the moment, I think we have "races"
with or without tasklist. Say, setpgrp() can add the new process to the
already "killed" pgrp.
Then, I think the only important case is SIGKILL/SIGSTOP (or other
signals which can't be blockes/ignored). We must kill/stop the entire
pgrp, we must not race with fork() and miss a child.
In this case I _think_ rcu_read_lock() is enough,
rcu_read_lock()
list_for_each_entry_rcu(task, pid->tasks[PIDTYPE_PGID)
group_send_sig_info(sig, task);
rcu_read_unlock();
except group_send_sig_info() can race with mt-exec, but this is simple
to fix.
If we send a signal (not necessary SIGKILL) to a process P, we must see
all childs which were forked by P, both send_signal() and copy_process()
take the same ->siglock, we must see the result of list_add_tail_rcu().
And, after we sent SIGKILL/SIGSTOP, it can't fork the new child.
If list_for_each_entry() does not see the exited process P, this means
we see the result of list_del_rcu(). But this also means we must the
the result of the previous list_add_rcu().
IOW, fork+exit means list_add_rcu() + wmb() + list_del_rcu(), if we
don't see the new entry on list, we must see the new one, right?
(I am ignoring the case when list_for_each_entry_rcu() sees a process
P but lock_task_sighand(P) fails, I think this is the same as if we
we missed P)
Now suppose a signal is blocked/ignored or has a handler. In this case
we can miss a child, but I think this is OK, we can pretend the new
child was forked after kill_pgrp() completes. Say, this child C was
forked by some process P. We can miss C only if it was forked after
we already sent the signal to P.
However. I do not pretend the reasoning above is "complete", and
perhaps I missed something else.
Not sure I understand... But afaics this case is covered above.
->siglock should serialize this, copy_process() does attach_pid()
under this lock.
Oleg.
--
