Re: [PATCH] fix send_sigqueue() vs thread exit race

From: Thomas Gleixner
Date: Sun Aug 21 2005 - 04:44:09 EST


On Sat, 2005-08-20 at 20:58 +0400, Oleg Nesterov wrote:
> [PATCH] fix send_sigqueue() vs thread exit race
>
> posix_timer_event() first checks that the thread (SIGEV_THREAD_ID
> case) does not have PF_EXITING flag, then it calls send_sigqueue()
> which locks task list. But if the thread exits in between the kernel
> will oops (->sighand == NULL after __exit_sighand).
>
> This patch moves the PF_EXITING check into the send_sigqueue(), it
> must be done atomically under tasklist_lock. When send_sigqueue()
> detects exiting thread it returns -1. In that case posix_timer_event
> will send the signal to thread group.
>
> Also, this patch fixes task_struct use-after-free in posix_timer_event.
>
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
>
> --- 2.6.13-rc6/kernel/signal.c~ 2005-08-18 23:10:28.000000000 +0400
> +++ 2.6.13-rc6/kernel/signal.c 2005-08-20 23:05:21.000000000 +0400
> @@ -1366,16 +1366,16 @@ send_sigqueue(int sig, struct sigqueue *
> unsigned long flags;
> int ret = 0;
>
> - /*
> - * We need the tasklist lock even for the specific
> - * thread case (when we don't need to follow the group
> - * lists) in order to avoid races with "p->sighand"
> - * going away or changing from under us.
> - */
> BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
> - read_lock(&tasklist_lock);
> + read_lock(&tasklist_lock);
> +
> + if (unlikely(p->flags & PF_EXITING)) {
> + ret = -1;
> + goto out_err;
> + }
> +

It's still racy. tasklist_lock does not protect anything here.

arm timer
exit
timer event
timr->it_process references a freed structure


tglx





-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/