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);
+ if (timer->it_process == current) {
+
+ if (timer->it_sigev_notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ timer->it_sigev_notify = SIGEV_SIGNAL;
+
+ timer->it_process = timer->it_process->group_leader;
+ }
+ spin_lock_irqrestore(&timer->it_lock, flags);
+ }
+}
+
+/*
* And now for the "clock" calls
*
* These functions are called both from timer functions (with the timer
diff -uprN --exclude-from=/usr/local/bin/diffit.exclude linux-2.6.13-rc6/kernel/signal.c linux-2.6.13-rc6.work/kernel/signal.c
--- linux-2.6.13-rc6/kernel/signal.c 2005-08-13 12:25:58.000000000 +0200
+++ linux-2.6.13-rc6.work/kernel/signal.c 2005-08-20 17:57:46.000000000 +0200
@@ -390,6 +390,7 @@ void __exit_signal(struct task_struct *t
sig->nvcsw += tsk->nvcsw;
sig->nivcsw += tsk->nivcsw;
sig->sched_time += tsk->sched_time;
+ exit_notify_itimers(sig);
spin_unlock(&sighand->siglock);
sig = NULL; /* Marker for below. */
}
@@ -1381,13 +1382,12 @@ send_sigqueue(int sig, struct sigqueue *
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.
+ * No need to hold tasklist lock here.
+ * posix_timer_event() is synchronized via
+ * exit_itimers() and exit_notify_itimers() to
+ * be protected against p->sighand == NULL
*/
BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
- read_lock(&tasklist_lock);
spin_lock_irqsave(&p->sighand->siglock, flags);
if (unlikely(!list_empty(&q->list))) {
@@ -1414,7 +1414,6 @@ send_sigqueue(int sig, struct sigqueue *
out:
spin_unlock_irqrestore(&p->sighand->siglock, flags);
- read_unlock(&tasklist_lock);
return(ret);
}
-
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/
