| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <55520589.9080401@oracle.com>
Date: Tue, 12 May 2015 09:52:09 -0400
From: Sasha Levin <sasha.levin@...cle.com>
To: linux-kernel@...r.kernel.org, pjt@...gle.com, tglx@...utronix.de,
klamm@...dex-team.ru, mingo@...nel.org, bsegall@...gle.com,
peterz@...radead.org, hpa@...or.com
Subject: Re: [tip:timers/core] hrtimer: Allow concurrent hrtimer_start() for
self restarting timers
On 04/22/2015 03:15 PM, tip-bot for Peter Zijlstra wrote:
> Commit-ID: 5de2755c8c8b3a6b8414870e2c284914a2b42e4d
> Gitweb: http://git.kernel.org/tip/5de2755c8c8b3a6b8414870e2c284914a2b42e4d
> Author: Peter Zijlstra <peterz@...radead.org>
> AuthorDate: Tue, 20 May 2014 15:49:48 +0200
> Committer: Thomas Gleixner <tglx@...utronix.de>
> CommitDate: Wed, 22 Apr 2015 17:06:52 +0200
>
> hrtimer: Allow concurrent hrtimer_start() for self restarting timers
>
> Because we drop cpu_base->lock around calling hrtimer::function, it is
> possible for hrtimer_start() to come in between and enqueue the timer.
>
> If hrtimer::function then returns HRTIMER_RESTART we'll hit the BUG_ON
> because HRTIMER_STATE_ENQUEUED will be set.
>
> Since the above is a perfectly valid scenario, remove the BUG_ON and
> make the enqueue_hrtimer() call conditional on the timer not being
> enqueued already.
>
> NOTE: in that concurrent scenario its entirely common for both sites
> to want to modify the hrtimer, since hrtimers don't provide
> serialization themselves be sure to provide some such that the
> hrtimer::function and the hrtimer_start() caller don't both try and
> fudge the expiration state at the same time.
>
> To that effect, add a WARN when someone tries to forward an already
> enqueued timer, the most common way to change the expiry of self
> restarting timers. Ideally we'd put the WARN in everything modifying
> the expiry but most of that is inlines and we don't need the bloat.
>
> Fixes: 2d44ae4d7135 ("hrtimer: clean up cpu->base locking tricks")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Cc: Ben Segall <bsegall@...gle.com>
> Cc: Roman Gushchin <klamm@...dex-team.ru>
> Cc: Paul Turner <pjt@...gle.com>
> Link: http://lkml.kernel.org/r/20150415113105.GT5029@twins.programming.kicks-ass.net
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
> kernel/time/hrtimer.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
> index 3bac942..4adf320 100644
> --- a/kernel/time/hrtimer.c
> +++ b/kernel/time/hrtimer.c
> @@ -799,6 +799,9 @@ u64 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval)
> if (delta.tv64 < 0)
> return 0;
>
> + if (WARN_ON(timer->state & HRTIMER_STATE_ENQUEUED))
> + return 0;
> +
> if (interval.tv64 < hrtimer_resolution)
> interval.tv64 = hrtimer_resolution;
Hey Peter,
I seem to be hitting this warning with the latest -next (2015-05-11):
[3344291.055800] WARNING: CPU: 0 PID: 9422 at kernel/time/hrtimer.c:802 hrtimer_forward+0x1f9/0x330()
[3344291.057883] Modules linked in:
[3344291.058734] CPU: 0 PID: 9422 Comm: trinity-main Tainted: G W 4.1.0-rc3-next-20150511-sa
sha-00037-g87c65d4-dirty #2199
[3344291.061763] ffff880003a88000 00000000d4a58de9 ffff880021007c88 ffffffffabc833ac
[3344291.063726] 0000000000000000 0000000000000000 ffff880021007cd8 ffffffffa21f0a86
[3344291.065656] ffffffffae6936c8 ffffffffa2386319 ffff880021007cb8 ffff8800211f5f90
[3344291.067590] Call Trace:
[3344291.068085] <IRQ> [<ffffffffabc833ac>] dump_stack+0x4f/0x7b
[3344291.068949] [<ffffffffa21f0a86>] warn_slowpath_common+0xc6/0x120
[3344291.070382] [<ffffffffa21f0cca>] warn_slowpath_null+0x1a/0x20
[3344291.071078] [<ffffffffa2386319>] hrtimer_forward+0x1f9/0x330
[3344291.071834] [<ffffffffa24fc7a0>] perf_mux_hrtimer_handler+0x340/0x750
[3344291.072616] [<ffffffffa238830c>] __hrtimer_run_queues+0x30c/0x10d0
[3344291.075592] [<ffffffffa238a7ac>] hrtimer_interrupt+0x19c/0x480
[3344291.076340] [<ffffffffa20c1bef>] local_apic_timer_interrupt+0x6f/0xc0
[3344291.077156] [<ffffffffabcf3af3>] smp_trace_apic_timer_interrupt+0xe3/0x859
[3344291.077968] [<ffffffffabcf1df0>] trace_apic_timer_interrupt+0x70/0x80
[3344291.078729] <EOI> [<ffffffffa2620890>] ? arch_local_irq_restore+0x10/0x30
[3344291.079582] [<ffffffffa2626ff4>] __slab_alloc+0x704/0x790
[3344291.082482] [<ffffffffa2627324>] kmem_cache_alloc+0x2a4/0x3a0
[3344291.083944] [<ffffffffa27cdf6d>] proc_alloc_inode+0x1d/0x270
[3344291.084631] [<ffffffffa26d1a7b>] alloc_inode+0x5b/0x170
[3344291.085278] [<ffffffffa26d7501>] new_inode_pseudo+0x11/0xd0
[3344291.085950] [<ffffffffa27ce698>] proc_get_inode+0x18/0x580
[3344291.086615] [<ffffffffa27dd946>] proc_lookup_de+0xc6/0x160
[3344291.088717] [<ffffffffa27f168c>] proc_tgid_net_lookup+0x5c/0xa0
[3344291.089435] [<ffffffffa269f470>] lookup_real+0x90/0x120
[3344291.090071] [<ffffffffa26a0183>] __lookup_hash+0xe3/0x100
[3344291.092954] [<ffffffffa26a96d7>] walk_component+0x697/0xc10
[3344291.096353] [<ffffffffa26ae0cf>] path_lookupat+0x13f/0x380
[3344291.097727] [<ffffffffa26ae441>] filename_lookup+0x131/0x1f0
[3344291.098411] [<ffffffffa26b4d11>] user_path_at_empty+0xc1/0x160
[3344291.101545] [<ffffffffa26b4dc1>] user_path_at+0x11/0x20
[3344291.102274] [<ffffffffa268f131>] vfs_fstatat+0xb1/0x140
[3344291.105842] [<ffffffffa2690299>] SYSC_newfstatat+0x79/0xd0
[3344291.108737] [<ffffffffa269041e>] SyS_newfstatat+0xe/0x10
[3344291.109377] [<ffffffffabcf0ee2>] tracesys_phase2+0x88/0x8d
Thanks,
Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists