Thomas Gleixner wrote:
>
> On Sat, 2005-08-20 at 18:10 +0400, Oleg Nesterov wrote:
>
> > 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.
>
> > posix_timer_event() runs under k_itimer.it_lock, but this does not
> > help if that thread was not the only one in thread group, in this
> > case we don't call exit_itimers().
>
> I added exit_notify_itimers() for that case. It is synchronized vs.
> posix_timer_event() via the timer lock and therefor protects against the
> race described above.
>
> It solves the problem for the only user of send_sigqueue for now, but I
> think we should have a more general interface to such functionality to
> allow simple usage.
>
> tglx
>
> Signed-off-by: Thomas Gleixner <[EMAIL PROTECTED]>
>
> diff -uprN --exclude-from=/usr/local/bin/diffit.exclude
> linux-2.6.13-rc6/include/linux/sched.h
> linux-2.6.13-rc6.work/include/linux/sched.h
> --- linux-2.6.13-rc6/include/linux/sched.h 2005-08-13 12:25:56.000000000
> +0200
> +++ linux-2.6.13-rc6.work/include/linux/sched.h 2005-08-20 17:43:36.000000000
> +0200
> @@ -1051,6 +1051,7 @@ extern void __exit_signal(struct task_st
> extern void exit_sighand(struct task_struct *);
> extern void __exit_sighand(struct task_struct *);
> extern void exit_itimers(struct signal_struct *);
> +extern void exit_notify_itimers(struct signal_struct *);
>
> extern NORET_TYPE void do_group_exit(int);
>
> diff -uprN --exclude-from=/usr/local/bin/diffit.exclude
> linux-2.6.13-rc6/kernel/posix-timers.c
> linux-2.6.13-rc6.work/kernel/posix-timers.c
> --- linux-2.6.13-rc6/kernel/posix-timers.c 2005-08-13 12:25:58.000000000
> +0200
> +++ linux-2.6.13-rc6.work/kernel/posix-timers.c 2005-08-20 17:43:36.000000000
> +0200
> @@ -1169,6 +1169,33 @@ void exit_itimers(struct signal_struct *
> }
>
> /*
> + * This is called by __exit_signal, when there are still references to
> + * the shared signal_struct
> + */
> +void exit_notify_itimers(struct signal_struct *sig)
> +{
> + struct k_itimer *timer;
> + struct list_head *tmp;
> + unsigned long flags;
> +
> + list_for_each(tmp, &sig->posix_timers) {
> +
> + timer = list_entry(tmp, struct k_itimer, list);
> +
> + /* Protect against posix_timer_fn */
> + spin_lock_irqsave(&timer->it_lock, flags);
I think this is deadlockable. We already (release_task) hold
tasklist_lock here. What if this timer has no SIGEV_THREAD_ID ?
posix_timer_fn locks timer->it_lock first, then locks tasklist_lock
in send_group_sigqueue().
Also, sys_timer_delete() locks ->it_lock, then ->sighand.lock.
I think the better fix would be re-check ->sighand in send_sigqueue,
like Paul's patches do.
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/
