On Sun, Nov 06, 2005 at 03:01:42PM +0300, Oleg Nesterov wrote:
> "Paul E. McKenney" wrote:
> >
> > So the idea is to error out of send_sigqueue() so that posix_timer_event()
> > will instead call send_group_sigqueeue(). But that could suffer from
> > the same race if the new leader thread also exits -- or if the exiting
> > thread was the leader thread to begin with.
>
> The case when leader exits is ok. If it is the only (last) thread - it will
> call exit_itimers(). If not - it (or sys_wait4 from parent) will not call
> release_task(), but will stay TASK_ZOMBIE with valid ->signal/sighand until
> the last thread in same thread group exits (and call exit_itimers).
>
> > But once send_group_sigqueue() read-acquires tasklist_lock, threads
> > and processes must stay put. So it should be possible to follow the
> > ->group_leader chain at that point.
>
> Not quite so, I think. See below.
>
> > Except that the group leader could do an exec(), right? If it does so,
> > it must do so before tasklist_lock is read-acquired. So the nightmare
> > case is where all but one thread exits, and then that one thread does
> > and exec().
>
> ... and that thread is not group leader. Actually, it does not matter
> if other threads exited or not, execing thread will kill other threads.
>
> > If this case can really happen, we want to drop the signal
> > on the floor, right?
>
> I think yes.
>
> > diff -urpNa -X dontdiff linux-2.6.14-mm0-fix/kernel/signal.c linux-2.6.14-mm0-fix-2/kernel/signal.c
> > --- linux-2.6.14-mm0-fix/kernel/signal.c 2005-11-04 17:23:40.000000000 -0800
> > +++ linux-2.6.14-mm0-fix-2/kernel/signal.c 2005-11-05 15:05:38.000000000 -0800
> > @@ -1408,6 +1408,11 @@ send_sigqueue(int sig, struct sigqueue *
> >
> > retry:
> > sh = rcu_dereference(p->sighand);
> > + if (sh == NULL) {
> > + /* We raced with pthread_exit()... */
> > + ret = -1;
> > + goto out_err;
> > + }
>
> I lost the plot. Because I can't apply this and previous patches (rejects)
> and can't imagine how send_sigqueue() looks now. I think this is ok, but
> we also need to re-check ->signal != NULL after lock(->sighand) or check
> PF_EXITING (iirc ve do have such check).
I lost the plot as well. There were apparently a very large number of
changes awaiting 2.6.14 coming out. ;-)
I also believe we have such a check.
> > @@ -1474,7 +1479,8 @@ send_group_sigqueue(int sig, struct sigq
> > BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
> >
> > read_lock(&tasklist_lock);
> > - /* Since it_lock is held, p->sighand cannot be NULL. */
> > + while (p->group_leader != p)
> > + p = p->group_leader;
>
> No, this is definitely not right. de_thread() does not change leader->group_leader
> when non-leader execs, so p->group_leader == p always.
This was intended for the case where the group leader does pthread_exit,
which would cause some other thread to assume group leadership. Or am
I missing something from that code path? (Quite likely that I am...)
Thanx, Paul
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
