diff options
| author | Oleg Nesterov <oleg@tv-sign.ru> | 2008-07-23 18:52:05 +0200 |
|---|---|---|
| committer | Thomas Gleixner <tglx@linutronix.de> | 2008-07-24 01:26:08 +0200 |
| commit | ba661292a2bc6ddd305a212b0526e5dc22195fe7 (patch) | |
| tree | 69d5749d967c67deffb6d804f61c359f5da928ad /kernel/signal.c | |
| parent | posix-timers: do_schedule_next_timer: fix the setting of ->si_overrun (diff) | |
| download | linux-ba661292a2bc6ddd305a212b0526e5dc22195fe7.tar.xz linux-ba661292a2bc6ddd305a212b0526e5dc22195fe7.zip | |
posix-timers: fix posix_timer_event() vs dequeue_signal() race
The bug was reported and analysed by Mark McLoughlin <markmc@redhat.com>,
the patch is based on his and Roland's suggestions.
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.
As Roland rightly pointed out, we need more cleanups/fixes here, see the
"FIXME" comment in the patch. Hopefully this patch makes sense anyway, and
it can mask the most bad implications.
Reported-by: Mark McLoughlin <markmc@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
Cc: Mark McLoughlin <markmc@redhat.com>
Cc: Oliver Pinter <oliver.pntr@gmail.com>
Cc: Roland McGrath <roland@redhat.com>
Cc: stable@kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
kernel/posix-timers.c | 17 +++++++++++++----
kernel/signal.c | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
Diffstat (limited to 'kernel/signal.c')
| -rw-r--r-- | kernel/signal.c | 1 |
1 files changed, 1 insertions, 0 deletions
diff --git a/kernel/signal.c b/kernel/signal.c index 72bb4f51f963..13fab9838354 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1280,6 +1280,7 @@ int send_sigqueue(struct sigqueue *q, struct task_struct *t, int group) q->info.si_overrun++; goto out; } + q->info.si_overrun = 0; signalfd_notify(t, sig); pending = group ? &t->signal->shared_pending : &t->pending; |