| 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: <1442584463-69553-1-git-send-email-dvyukov@google.com>
Date: Fri, 18 Sep 2015 15:54:23 +0200
From: Dmitry Vyukov <dvyukov@...gle.com>
To: john.stultz@...aro.org, tglx@...utronix.de,
linux-kernel@...r.kernel.org
Cc: andreyknvl@...gle.com, glider@...gle.com, kcc@...gle.com,
ktsan@...glegroups.com, Dmitry Vyukov <dvyukov@...gle.com>
Subject: [PATCH] kernel/time: fix data race in timer_stats_account_timer
timer_stats_account_timer() reads timer->start_site,
then checks it for NULL and then re-reads it again.
While timer_stats_timer_clear_start_info() can concurrently
reset timer->start_site to NULL. This should not lead to
crashes, but can double number of entries in timer stats
as start_site is used during comparison, the doubled entries
will have unuseful NULL start_site.
Read timer->start_site only once in timer_stats_account_timer().
The data race was found with KernelThreadSanitizer (KTSAN).
Signed-off-by: Dmitry Vyukov <dvyukov@...gle.com>
---
The race report for the record (on 4.2 rc2):
ThreadSanitizer: data-race in run_timer_softirq
Read at 0xffff8804834f0d08 of size 8 by thread 1011 on CPU 8:
[< inline >] timer_stats_account_timer kernel/time/timer.c:464
[< inline >] __run_timers kernel/time/timer.c:1220
[<ffffffff8110f2dc>] run_timer_softirq+0x1ac/0x500 kernel/time/timer.c:1414
[<ffffffff81091c1e>] __do_softirq+0xbe/0x2f0 kernel/softirq.c:273
[<ffffffff81ee4cca>] apic_timer_interrupt+0x8a/0xa0 arch/x86/entry/entry_64.S:790
Previous write at 0xffff8804834f0d08 of size 8 by thread 1012 on CPU 7:
[< inline >] timer_stats_timer_clear_start_info include/linux/timer.h:208
[<ffffffff8110e585>] del_timer+0x25/0x80 kernel/time/timer.c:1005
[< inline >] cfq_del_timer block/cfq-iosched.c:2497
[<ffffffff81541552>] __cfq_slice_expired+0x3d2/0x990 block/cfq-iosched.c:2537
[< inline >] cfq_slice_expired block/cfq-iosched.c:2583
[< inline >] cfq_preempt_queue block/cfq-iosched.c:3935
[< inline >] cfq_rq_enqueued block/cfq-iosched.c:4005
[<ffffffff81545aee>] cfq_insert_request+0x95e/0xc70 block/cfq-iosched.c:4023
[<ffffffff81509ab3>] __elv_add_request+0x293/0x4e0 block/elevator.c:659
[<ffffffff81516387>] blk_flush_plug_list+0x3a7/0x420 block/blk-core.c:3204
[<ffffffff81516974>] blk_finish_plug+0x54/0x80 block/blk-core.c:3222
[< inline >] read_pages mm/readahead.c:139
[<ffffffff811e0e8b>] __do_page_cache_readahead+0x29b/0x3a0 mm/readahead.c:200
[<ffffffff811e10e2>] ondemand_readahead+0x152/0x370 mm/readahead.c:454
[<ffffffff811e15c6>] page_cache_sync_readahead+0x66/0xa0 mm/readahead.c:504
[< inline >] do_generic_file_read mm/filemap.c:1516
[<ffffffff811ce712>] generic_file_read_iter+0x712/0x880 mm/filemap.c:1759
[< inline >] new_sync_read fs/read_write.c:422
[<ffffffff8125f530>] __vfs_read+0x1a0/0x1e0 fs/read_write.c:434
[<ffffffff81260337>] vfs_read+0xb7/0x1e0 fs/read_write.c:454
[<ffffffff81269429>] kernel_read+0x79/0xb0 fs/exec.c:821
[<ffffffff81269a2d>] prepare_binprm+0x15d/0x2b0 fs/exec.c:1347
[<ffffffff8126c590>] do_execveat_common.isra.36+0x820/0xb40 fs/exec.c:1567
[< inline >] do_execve fs/exec.c:1628
[< inline >] SYSC_execve fs/exec.c:1709
[<ffffffff8126cdc6>] SyS_execve+0x46/0x60 fs/exec.c:1704
[<ffffffff81ee4145>] return_from_execve+0x0/0x23 arch/x86/entry/entry_64.S:428
Mutexes locked by thread 1011:
Mutex 2498 is locked here:
[< inline >] __raw_spin_lock_irq include/linux/spinlock_api_smp.h:140
[<ffffffff81ee375c>] _raw_spin_lock_irq+0x5c/0x80 kernel/locking/spinlock.c:167
[< inline >] spin_lock_irq include/linux/spinlock.h:342
[< inline >] __run_timers kernel/time/timer.c:1186
[<ffffffff8110f198>] run_timer_softirq+0x68/0x500 kernel/time/timer.c:1414
[<ffffffff81091c1e>] __do_softirq+0xbe/0x2f0 kernel/softirq.c:273
[<ffffffff81ee4cca>] apic_timer_interrupt+0x8a/0xa0 arch/x86/entry/entry_64.S:790
Mutexes locked by thread 1012:
Mutex 128390 is locked here:
[<ffffffff81edf7c2>] mutex_lock_interruptible+0x62/0xa0 kernel/locking/mutex.c:805
[<ffffffff8126bd0f>] prepare_bprm_creds+0x4f/0xb0 fs/exec.c:1172
[<ffffffff8126beaf>] do_execveat_common.isra.36+0x13f/0xb40 fs/exec.c:1517
[< inline >] do_execve fs/exec.c:1628
[< inline >] SYSC_execve fs/exec.c:1709
[<ffffffff8126cdc6>] SyS_execve+0x46/0x60 fs/exec.c:1704
[<ffffffff81ee4145>] return_from_execve+0x0/0x23 arch/x86/entry/entry_64.S:428
Mutex 106756 is locked here:
[< inline >] __raw_spin_lock include/linux/spinlock_api_smp.h:158
[<ffffffff81ee37d0>] _raw_spin_lock+0x50/0x70 kernel/locking/spinlock.c:151
[< inline >] spin_lock include/linux/spinlock.h:312
[<ffffffff8151633f>] blk_flush_plug_list+0x35f/0x420 block/blk-core.c:3187
[<ffffffff81516974>] blk_finish_plug+0x54/0x80 block/blk-core.c:3222
[< inline >] read_pages mm/readahead.c:139
[<ffffffff811e0e8b>] __do_page_cache_readahead+0x29b/0x3a0 mm/readahead.c:200
[<ffffffff811e10e2>] ondemand_readahead+0x152/0x370 mm/readahead.c:454
[<ffffffff811e15c6>] page_cache_sync_readahead+0x66/0xa0 mm/readahead.c:504
[< inline >] do_generic_file_read mm/filemap.c:1516
[<ffffffff811ce712>] generic_file_read_iter+0x712/0x880 mm/filemap.c:1759
[< inline >] new_sync_read fs/read_write.c:422
[<ffffffff8125f530>] __vfs_read+0x1a0/0x1e0 fs/read_write.c:434
[<ffffffff81260337>] vfs_read+0xb7/0x1e0 fs/read_write.c:454
[<ffffffff81269429>] kernel_read+0x79/0xb0 fs/exec.c:821
[<ffffffff81269a2d>] prepare_binprm+0x15d/0x2b0 fs/exec.c:1347
[<ffffffff8126c590>] do_execveat_common.isra.36+0x820/0xb40 fs/exec.c:1567
[< inline >] do_execve fs/exec.c:1628
[< inline >] SYSC_execve fs/exec.c:1709
[<ffffffff8126cdc6>] SyS_execve+0x46/0x60 fs/exec.c:1704
[<ffffffff81ee4145>] return_from_execve+0x0/0x23 arch/x86/entry/entry_64.S:428
---
kernel/time/timer.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 5e097fa..96f7d1f 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -461,10 +461,15 @@ void __timer_stats_timer_set_start_info(struct timer_list *timer, void *addr)
static void timer_stats_account_timer(struct timer_list *timer)
{
- if (likely(!timer->start_site))
+ void *site;
+
+ /* start_site can be concurrently reset by
+ timer_stats_timer_clear_start_info() */
+ site = READ_ONCE(timer->start_site);
+ if (likely(!site))
return;
- timer_stats_update_stats(timer, timer->start_pid, timer->start_site,
+ timer_stats_update_stats(timer, timer->start_pid, site,
timer->function, timer->start_comm,
timer->flags);
}
--
2.6.0.rc0.131.gf624c3d
--
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