| 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
| ||
|
Date: Wed, 9 Sep 2015 12:24:50 +0200
From: Dmitry Vyukov <dvyukov@...gle.com>
To: akpm@...ux-foundation.org, linux@...musvillemoes.dk,
ying.huang@...el.com
Cc: linux-kernel@...r.kernel.org, kcc@...gle.com,
andreyknvl@...gle.com, glider@...gle.com, ktsan@...glegroups.com,
Dmitry Vyukov <dvyukov@...gle.com>
Subject: [PATCH] lib: fix data race in llist_del_first
llist_del_first reads entry->next, but it did not acquire
visibility over the entry node. As the result it can get
a stale value of entry->next (e.g. NULL or whatever garbage
was there before the appending thread wrote correct value).
And then commit that value as llist head with cmpxchg.
That will corrupt llist.
Note there is a control-dependency between read of head->first
and read of entry->next, but it does not make the code correct.
Kernel memory model unambiguously says:
"A load-load control dependency requires a full read memory barrier".
Use smp_load_acquire to acquire visibility over the entry node.
The data race was found with KernelThreadSanitizer (KTSAN).
Signed-off-by: Dmitry Vyukov <dvyukov@...gle.com>
---
Here is an example of KTSAN report:
ThreadSanitizer: data-race in llist_del_first
Read of size 1 by thread T389 (K2630, CPU0):
[<ffffffff8156b8a9>] llist_del_first+0x39/0x70 lib/llist.c:74
[< inlined >] tty_buffer_alloc drivers/tty/tty_buffer.c:181
[<ffffffff81664af4>] __tty_buffer_request_room+0xb4/0x250 drivers/tty/tty_buffer.c:292
[<ffffffff81664e6c>] tty_insert_flip_string_fixed_flag+0x6c/0x150 drivers/tty/tty_buffer.c:337
[< inlined >] tty_insert_flip_string include/linux/tty_flip.h:35
[<ffffffff81667422>] pty_write+0x72/0xc0 drivers/tty/pty.c:110
[< inlined >] process_output_block drivers/tty/n_tty.c:611
[<ffffffff8165c016>] n_tty_write+0x346/0x7f0 drivers/tty/n_tty.c:2401
[< inlined >] do_tty_write drivers/tty/tty_io.c:1159
[<ffffffff816568df>] tty_write+0x21f/0x3f0 drivers/tty/tty_io.c:1245
[<ffffffff8125f00f>] __vfs_write+0x5f/0x1f0 fs/read_write.c:489
[<ffffffff8125ff8f>] vfs_write+0xef/0x280 fs/read_write.c:538
[< inlined >] SYSC_write fs/read_write.c:585
[<ffffffff81261390>] SyS_write+0x70/0xe0 fs/read_write.c:577
[<ffffffff81ee862e>] entry_SYSCALL_64_fastpath+0x12/0x71 arch/x86/entry/entry_64.S:186
Previous write of size 8 by thread T226 (K761, CPU0):
[<ffffffff8156b832>] llist_add_batch+0x32/0x70 lib/llist.c:44 (discriminator 16)
[< inlined >] llist_add include/linux/llist.h:180
[<ffffffff816649fc>] tty_buffer_free+0x6c/0xb0 drivers/tty/tty_buffer.c:221
[<ffffffff816651e7>] flush_to_ldisc+0x107/0x300 drivers/tty/tty_buffer.c:514
[<ffffffff810b20ee>] process_one_work+0x47e/0x930 kernel/workqueue.c:2036
[<ffffffff810b2650>] worker_thread+0xb0/0x900 kernel/workqueue.c:2170
[<ffffffff810bbe20>] kthread+0x150/0x170 kernel/kthread.c:209
[<ffffffff81ee8a1f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:526
---
lib/llist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/llist.c b/lib/llist.c
index 0b0e977..c103205 100644
--- a/lib/llist.c
+++ b/lib/llist.c
@@ -66,12 +66,12 @@ struct llist_node *llist_del_first(struct llist_head *head)
{
struct llist_node *entry, *old_entry, *next;
- entry = head->first;
+ entry = READ_ONCE_CTRL(head->first);
for (;;) {
if (entry == NULL)
return NULL;
old_entry = entry;
- next = entry->next;
+ next = READ_ONCE(entry->next);
entry = cmpxchg(&head->first, old_entry, next);
if (entry == old_entry)
break;
--
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