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

[Thomas Gleixner <tglx@linutronix.de>]

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@tv-sign.ru>
> 
> --- 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