"Paul E. McKenney" wrote:
>
> @@ -1386,7 +1387,7 @@ send_sigqueue(int sig, struct sigqueue *
> {
> unsigned long flags;
> int ret = 0;
> - struct sighand_struct *sh = p->sighand;
> + struct sighand_struct *sh;
>
> BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
>
> @@ -1405,7 +1406,15 @@ send_sigqueue(int sig, struct sigqueue *
> goto out_err;
> }
>
> +retry:
> + sh = rcu_dereference(p->sighand);
> +
> spin_lock_irqsave(&sh->siglock, flags);
> + if (p->sighand != sh) {
> + /* We raced with exec() in a multithreaded process... */
> + spin_unlock_irqrestore(&sh->siglock, flags);
> + goto retry;
p->sighand can't be changed, de_thread calls exit_itimers() before
changing ->sighand. But I still think it can be NULL, and send_sigqueue()
should return -1 in that case.
> @@ -1464,15 +1473,8 @@ send_group_sigqueue(int sig, struct sigq
>
> BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
>
> - while (!read_trylock(&tasklist_lock)) {
> - if (!p->sighand)
> - return -1;
> - cpu_relax();
> - }
> - if (unlikely(!p->sighand)) {
> - ret = -1;
> - goto out_err;
> - }
> + read_lock(&tasklist_lock);
> + /* Since it_lock is held, p->sighand cannot be NULL. */
> spin_lock_irqsave(&p->sighand->siglock, flags);
Again, I think the comment is wrong.
However, now I think we really have a race with exec, and this race was not
introduced by your patches!
If !thread_group_leader() does exec de_thread() calls release_task(->group_leader)
before calling exit_itimers(). This means that send_group_sigqueue() which
always has p == ->group_leader parameter can oops here.
Oleg.
-
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]
