Commit 60187d27 authored by Oleg Nesterov's avatar Oleg Nesterov Committed by Linus Torvalds

sigqueue_free: fix the race with collect_signal()

Spotted by taoyue <yue.tao@windriver.com> and Jeremy Katz <jeremy.katz@windriver.com>.

collect_signal:				sigqueue_free:

	list_del_init(&first->list);
						if (!list_empty(&q->list)) {
							// not taken
						}
						q->flags &= ~SIGQUEUE_PREALLOC;

	__sigqueue_free(first);			__sigqueue_free(q);

Now, __sigqueue_free() is called twice on the same "struct sigqueue" with the
obviously bad implications.

In particular, this double free breaks the array_cache->avail logic, so the
same sigqueue could be "allocated" twice, and the bug can manifest itself via
the "impossible" BUG_ON(!SIGQUEUE_PREALLOC) in sigqueue_free/send_sigqueue.

Hopefully this can explain these mysterious bug-reports, see

	http://marc.info/?t=118766926500003
	http://marc.info/?t=118466273000005

Alexey Dobriyan reports this patch makes the difference for the testcase, but
nobody has an access to the application which opened the problems originally.

Also, this patch removes tasklist lock/unlock, ->siglock is enough.
Signed-off-by: default avatarOleg Nesterov <oleg@tv-sign.ru>
Cc: taoyue <yue.tao@windriver.com>
Cc: Jeremy Katz <jeremy.katz@windriver.com>
Cc: Sukadev Bhattiprolu <sukadev@us.ibm.com>
Cc: Alexey Dobriyan <adobriyan@sw.ru>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Roland McGrath <roland@redhat.com>
Cc: <stable@kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 99db67bc
...@@ -1300,20 +1300,19 @@ struct sigqueue *sigqueue_alloc(void) ...@@ -1300,20 +1300,19 @@ struct sigqueue *sigqueue_alloc(void)
void sigqueue_free(struct sigqueue *q) void sigqueue_free(struct sigqueue *q)
{ {
unsigned long flags; unsigned long flags;
spinlock_t *lock = &current->sighand->siglock;
BUG_ON(!(q->flags & SIGQUEUE_PREALLOC)); BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
/* /*
* If the signal is still pending remove it from the * If the signal is still pending remove it from the
* pending queue. * pending queue. We must hold ->siglock while testing
* q->list to serialize with collect_signal().
*/ */
if (unlikely(!list_empty(&q->list))) { spin_lock_irqsave(lock, flags);
spinlock_t *lock = &current->sighand->siglock; if (!list_empty(&q->list))
read_lock(&tasklist_lock); list_del_init(&q->list);
spin_lock_irqsave(lock, flags); spin_unlock_irqrestore(lock, flags);
if (!list_empty(&q->list))
list_del_init(&q->list);
spin_unlock_irqrestore(lock, flags);
read_unlock(&tasklist_lock);
}
q->flags &= ~SIGQUEUE_PREALLOC; q->flags &= ~SIGQUEUE_PREALLOC;
__sigqueue_free(q); __sigqueue_free(q);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment