From: Mark McLoughlin <markmc@redhat.com>
Date: Wed, 8 Oct 2008 21:24:55 +0100
Subject: [misc] posix-timers: event vs dequeue_signal() race
Message-id: 1223497495.12947.14.camel@blaa
O-Subject: [RHEL5.3 PATCH] posix-timers: fix posix_timer_event() vs dequeue_signal() race
Bugzilla: 466167
RH-Acked-by: Brian Maly <bmaly@redhat.com>
RH-Acked-by: Chris Wright <chrisw@redhat.com>
RH-Acked-by: Jiri Pirko <jpirko@redhat.com>

https://bugzilla.redhat.com/466167

commit ba661292a2bc6ddd305a212b0526e5dc22195fe7 upstream

posix_timer_event() always rewrites the pre-allocated siginfo before sending
the signal. Most of the written info is the same all the time, but memset(0)
is very wrong. If ->sigq is queued we can race with collect_signal() which
can fail to find this siginfo looking at .si_signo, or copy_siginfo() can
copy the wrong .si_code/si_tid/etc.

In short, sys_timer_settime() can in fact stop the active timer, or the user
can receive the siginfo with the wrong .si_xxx values.

Move "memset(->info, 0)" from posix_timer_event() to alloc_posix_timer(),
change send_sigqueue() to set .si_overrun = 0 when ->sigq is not queued.
It would be nice to move the whole sigq->info initialization from send to
create path, but this is not easy to do without uglifying timer_create()
further.

This issue has been observed by causing KVM networking to occasionally die when
running on RHEL5 kernels.

The fix is upstream in 2.6.27, 2.6.26.2 and 2.6.25.16. Dor Laor could easily
reproduce on RHEL5 and has confirmed that the patch fixes it for him.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>

 kernel/posix-timers.c |   17 +++++++++++++----
 kernel/signal.c       |    1 +
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index ac6dc87..3f52c08 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -296,14 +296,22 @@ void do_schedule_next_timer(struct siginfo *info)
 		unlock_timer(timr, flags);
 }
 
-int posix_timer_event(struct k_itimer *timr,int si_private)
+int posix_timer_event(struct k_itimer *timr, int si_private)
 {
-	memset(&timr->sigq->info, 0, sizeof(siginfo_t));
+	/*
+	 * FIXME: if ->sigq is queued we can race with
+	 * dequeue_signal()->do_schedule_next_timer().
+	 *
+	 * If dequeue_signal() sees the "right" value of
+	 * si_sys_private it calls do_schedule_next_timer().
+	 * We re-queue ->sigq and drop ->it_lock().
+	 * do_schedule_next_timer() locks the timer
+	 * and re-schedules it while ->sigq is pending.
+	 * Not really bad, but not that we want.
+	 */
 	timr->sigq->info.si_sys_private = si_private;
-	/* Send signal to the process that owns this timer.*/
 
 	timr->sigq->info.si_signo = timr->it_sigev_signo;
-	timr->sigq->info.si_errno = 0;
 	timr->sigq->info.si_code = SI_TIMER;
 	timr->sigq->info.si_tid = timr->it_id;
 	timr->sigq->info.si_value = timr->it_sigev_value;
@@ -407,6 +415,7 @@ static struct k_itimer * alloc_posix_timer(void)
 		kmem_cache_free(posix_timers_cache, tmr);
 		tmr = NULL;
 	}
+	memset(&tmr->sigq->info, 0, sizeof(siginfo_t));
 	return tmr;
 }
 
diff --git a/kernel/signal.c b/kernel/signal.c
index 4c8010b..e1296e6 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1276,6 +1276,7 @@ int send_sigqueue(int sig, struct sigqueue *q, struct task_struct *p)
 		ret = -1;
 		goto out_err;
 	}
+	q->info.si_overrun = 0;
 
 	if (unlikely(!list_empty(&q->list))) {
 		/*