From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754286AbbF0LBM (ORCPT <rfc822;w@1wt.eu>);
	Sat, 27 Jun 2015 07:01:12 -0400
Received: from mail.skyhub.de ([78.46.96.112]:33851 "EHLO mail.skyhub.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752633AbbF0LBE (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Sat, 27 Jun 2015 07:01:04 -0400
Date: Sat, 27 Jun 2015 13:00:46 +0200
From: Borislav Petkov <bp@alien8.de>
To: Jan Kiszka <jan.kiszka@web.de>
Cc: Thomas Gleixner <tglx@linutronix.de>, LKML <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>, Peter Zijlstra <peterz@infradead.org>,
        Paul McKenney <paulmck@linux.vnet.ibm.com>,
        Frederic Weisbecker <fweisbec@gmail.com>,
        Eric Dumazet <edumazet@google.com>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        John Stultz <john.stultz@linaro.org>,
        Joonwoo Park <joonwoop@codeaurora.org>,
        Wenbo Wang <wenbo.wang@memblaze.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Badhri Jagan Sridharan <Badhri@google.com>
Subject: Re: [PATCH] timer: Fix unsafe cpu variable access in migrate_timers
Message-ID: <20150627110046.GC27131@pd.tnic>
References: <20150526210723.245729529@linutronix.de>
 <20150526224511.950084301@linutronix.de>
 <558E7325.70505@web.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <558E7325.70505@web.de>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Jun 27, 2015 at 11:55:49AM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> migrate_timers is invoked with preemption enabled. Therefore we have to
> get/put the cpu-local variable tvec_bases like before commit 0eeda71bc3.
> 
> This fixes
> 
> BUG: using smp_processor_id() in preemptible [00000000] code: bash/4917
> caller is debug_smp_processor_id+0x17/0x19
> CPU: 0 PID: 4917 Comm: bash Not tainted 4.1.0-dbg+ #97
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.8.1-0-g4adadbd-20150316_085822-nilsson.home.kraxel.org 04/01/2014
>  ffff880038674040 ffff880002e6fb98 ffffffff815356a0 0000000000000002
>  0000000000000000 ffff880002e6fbc8 ffffffff8130c16f ffff88003fd8d500
>  000000000000d500 0000000000000000 0000000000000003 ffff880002e6fbd8
> Call Trace:
>  [<ffffffff815356a0>] dump_stack+0x4f/0x7b
>  [<ffffffff8130c16f>] check_preemption_disabled+0xdd/0xef
>  [<ffffffff8130c198>] debug_smp_processor_id+0x17/0x19
>  [<ffffffff810a9b37>] timer_cpu_notify+0x4f/0x112
>  [<ffffffff8106718b>] notifier_call_chain+0x4c/0x71
>  [<ffffffff810671be>] __raw_notifier_call_chain+0xe/0x10
>  [<ffffffff810484b6>] __cpu_notify+0x20/0x37
>  [<ffffffff810484e0>] cpu_notify+0x13/0x15
>  [<ffffffff81048591>] cpu_notify_nofail+0xe/0x16
>  [<ffffffff8152efb2>] _cpu_down+0x178/0x268
>  [<ffffffff8108bc3a>] ? trace_hardirqs_on+0xd/0xf
>  [<ffffffff8152f0ca>] cpu_down+0x28/0x3c
>  [<ffffffff813cbdb9>] cpu_subsys_offline+0x14/0x16
>  [<ffffffff813c779d>] device_offline+0x7d/0xb1
>  [<ffffffff813c78a2>] online_store+0x48/0x68
>  [<ffffffff813c5544>] dev_attr_store+0x18/0x22
>  [<ffffffff811dac6c>] sysfs_kf_write+0x49/0x51
>  [<ffffffff811da139>] kernfs_fop_write+0x105/0x158
>  [<ffffffff8116c54f>] __vfs_write+0x28/0xbd
>  [<ffffffff812ab014>] ? security_file_permission+0x23/0x90
>  [<ffffffff8116ccb2>] vfs_write+0xb2/0x169
>  [<ffffffff8116d717>] SyS_write+0x4a/0x91
>  [<ffffffff8153d12e>] entry_SYSCALL_64_fastpath+0x12/0x76
> 
> triggered when offlining a CPU, e.g. via sysfs.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  kernel/time/timer.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 520499d..c826178 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -1566,7 +1566,7 @@ static void migrate_timers(int cpu)
>  
>  	BUG_ON(cpu_online(cpu));
>  	old_base = per_cpu_ptr(&tvec_bases, cpu);
> -	new_base = this_cpu_ptr(&tvec_bases);
> +	new_base = &get_cpu_var(tvec_bases);

Hmm, tglx's version doesn't disable preemtion around it:

http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/urgent&id=24bfcb100959c8641a627b5604d967243f8f240c

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--