Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756245AbbKDTfr (ORCPT ); Wed, 4 Nov 2015 14:35:47 -0500 Received: from www.linutronix.de ([62.245.132.108]:51335 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756134AbbKDTfp (ORCPT ); Wed, 4 Nov 2015 14:35:45 -0500 Date: Wed, 4 Nov 2015 20:35:00 +0100 (CET) From: Thomas Gleixner To: Tejun Heo cc: Chris Worley , linux-kernel@vger.kernel.org, bfields@fieldses.org, Michael Skralivetsky , Trond Myklebust , Shaohua Li , Jeff Layton , kernel-team@fb.com Subject: Re: [PATCH] timer: add_timer_on() should perform proper migration In-Reply-To: <20151104171533.GI5749@mtj.duckdns.org> Message-ID: References: <20151031073400.2cf05d77@tlielax.poochiereds.net> <20151031213107.GA23841@mtj.duckdns.org> <20151031175404.3c57a17a@tlielax.poochiereds.net> <20151102145633.5329f3da@tlielax.poochiereds.net> <20151102203339.7ed8f2bb@synchrony.poochiereds.net> <20151103125504.6649138f@tlielax.poochiereds.net> <20151103225405.GG5749@mtj.duckdns.org> <20151104000658.GH5749@mtj.duckdns.org> <20151104064836.661b0e01@tlielax.poochiereds.net> <20151104171533.GI5749@mtj.duckdns.org> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1831 Lines: 63 Tejun, On Wed, 4 Nov 2015, Tejun Heo wrote: > Regardless of the previous CPU a timer was on, add_timer_on() > currently simply sets timer->flags to the new CPU. As the caller must > be seeing the timer as idle, this is locally fine, but the timer > leaving the old base while unlocked can lead to race conditions as > follows. nice detective work. This has been there forever. I really wonder why nobody ever triggered this before. @stable: The patch does only apply to kernels >= 4.2. Backport for older kernels is below. Thanks, tglx -----------> --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -966,13 +966,26 @@ EXPORT_SYMBOL(add_timer); */ void add_timer_on(struct timer_list *timer, int cpu) { - struct tvec_base *base = per_cpu(tvec_bases, cpu); + struct tvec_base *new_base = per_cpu(tvec_bases, cpu); + struct tvec_base *base; unsigned long flags; timer_stats_timer_set_start_info(timer); BUG_ON(timer_pending(timer) || !timer->function); - spin_lock_irqsave(&base->lock, flags); - timer_set_base(timer, base); + + /* + * If @timer was on a different CPU, it should be migrated with the + * old base locked to prevent other operations proceeding with the + * wrong base locked. See lock_timer_base(). + */ + base = lock_timer_base(timer, &flags); + if (base != new_base) { + timer_set_base(timer, NULL); + spin_unlock(&base->lock); + base = new_base; + spin_lock(&base->lock); + timer_set_base(timer, base); + } debug_activate(timer, timer->expires); internal_add_timer(base, timer); spin_unlock_irqrestore(&base->lock, flags); -- 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/