Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760796AbXIXQ1P (ORCPT ); Mon, 24 Sep 2007 12:27:15 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1760407AbXIXQZA (ORCPT ); Mon, 24 Sep 2007 12:25:00 -0400 Received: from canuck.infradead.org ([209.217.80.40]:47203 "EHLO canuck.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760397AbXIXQY6 (ORCPT ); Mon, 24 Sep 2007 12:24:58 -0400 Date: Mon, 24 Sep 2007 09:20:04 -0700 From: Greg KH To: linux-kernel@vger.kernel.org, stable@kernel.org, torvalds@linux-foundation.org Cc: Justin Forbes , Zwane Mwaikambo , "Theodore Ts'o" , Randy Dunlap , Dave Jones , Chuck Wolber , Chris Wedgwood , Michael Krufky , Chuck Ebbert , Domenico Andreoli , akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk, oleg@tv-sign.ru, sukadev@us.ibm.com, adobriyan@sw.ru, tglx@linutronix.de, jeremy.katz@windriver.com, yue.tao@windriver.com, mingo@elte.hu, roland@redhat.com Subject: [06/50] sigqueue_free: fix the race with collect_signal() Message-ID: <20070924162004.GG13510@kroah.com> References: <20070924161246.983665021@mini.kroah.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="sigqueue_free-fix-the-race-with-collect_signal.patch" In-Reply-To: <20070924161733.GA13510@kroah.com> User-Agent: Mutt/1.5.16 (2007-06-09) X-Bad-Reply: References and In-Reply-To but no 'Re:' in Subject. Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2886 Lines: 89 From: Oleg Nesterov commit 60187d2708caa870f0825d753df1612ea688eb9e in mainline. Spotted by taoyue and Jeremy Katz . 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: Oleg Nesterov Cc: taoyue Cc: Jeremy Katz Cc: Sukadev Bhattiprolu Cc: Alexey Dobriyan Cc: Ingo Molnar Cc: Thomas Gleixner Cc: Roland McGrath Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman --- kernel/signal.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1259,20 +1259,19 @@ struct sigqueue *sigqueue_alloc(void) void sigqueue_free(struct sigqueue *q) { unsigned long flags; + spinlock_t *lock = ¤t->sighand->siglock; + BUG_ON(!(q->flags & SIGQUEUE_PREALLOC)); /* * 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))) { - spinlock_t *lock = ¤t->sighand->siglock; - read_lock(&tasklist_lock); - spin_lock_irqsave(lock, flags); - if (!list_empty(&q->list)) - list_del_init(&q->list); - spin_unlock_irqrestore(lock, flags); - read_unlock(&tasklist_lock); - } + spin_lock_irqsave(lock, flags); + if (!list_empty(&q->list)) + list_del_init(&q->list); + spin_unlock_irqrestore(lock, flags); + q->flags &= ~SIGQUEUE_PREALLOC; __sigqueue_free(q); } -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/