| 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: <1303293765.3186.74.camel@edumazet-laptop>
Date: Wed, 20 Apr 2011 12:02:45 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: Pekka Enberg <penberg@...helsinki.fi>
Cc: casteyde.christian@...e.fr, Christoph Lameter <cl@...ux.com>,
Andrew Morton <akpm@...ux-foundation.org>,
netdev@...r.kernel.org, bugzilla-daemon@...zilla.kernel.org,
bugme-daemon@...zilla.kernel.org,
Vegard Nossum <vegardno@....uio.no>
Subject: Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from
uninitialized memory in __alloc_skb
Le mercredi 20 avril 2011 à 11:07 +0200, Eric Dumazet a écrit :
> I had one splat (before applying any patch), adding CONFIG_PREEMPT to my
> build :
>
> [ 84.629936] WARNING: kmemcheck: Caught 64-bit read from uninitialized memory (ffff88011a7376f0)
> [ 84.629939] 5868ba1a0188ffff5868ba1a0188ffff7078731a0188ffffe0e1181a0188ffff
> [ 84.629952] i i i i i i i i i i i i i i i i u u u u u u u u u u u u u u u u
> [ 84.629963] ^
> [ 84.629964]
> [ 84.629966] Pid: 2060, comm: 05-wait_for_sys Not tainted 2.6.39-rc4-00237-ge3de956-dirty #550 HP ProLiant BL460c G6
> [ 84.629969] RIP: 0010:[<ffffffff810f0e80>] [<ffffffff810f0e80>] kmem_cache_alloc+0x50/0x190
> [ 84.629977] RSP: 0018:ffff88011a0b9988 EFLAGS: 00010286
> [ 84.629979] RAX: 0000000000000000 RBX: ffff88011a739a98 RCX: 0000000000620780
> [ 84.629980] RDX: 0000000000620740 RSI: 0000000000017400 RDI: ffffffff810db8f5
> [ 84.629982] RBP: ffff88011a0b99b8 R08: ffff88011a261dd8 R09: 0000000000000009
> [ 84.629983] R10: 0000000000000002 R11: ffff88011fffce00 R12: ffff88011b00a600
> [ 84.629985] R13: ffff88011a7376f0 R14: 00000000000000d0 R15: ffff88011abbb0c0
> [ 84.629987] FS: 0000000000000000(0000) GS:ffff88011fc00000(0063) knlGS:00000000f76ff6c0
> [ 84.629989] CS: 0010 DS: 002b ES: 002b CR0: 000000008005003b
> [ 84.629991] CR2: ffff88011998dab8 CR3: 000000011a13f000 CR4: 00000000000006f0
> [ 84.629992] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 84.629994] DR3: 0000000000000000 DR6: 00000000ffff4ff0 DR7: 0000000000000400
> [ 84.629995] [<ffffffff810db8f5>] anon_vma_prepare+0x55/0x190
> [ 84.630000] [<ffffffff810cfe90>] handle_pte_fault+0x470/0x6e0
> [ 84.630002] [<ffffffff810d1654>] handle_mm_fault+0x124/0x1a0
> [ 84.630005] [<ffffffff8147a510>] do_page_fault+0x160/0x560
> [ 84.630008] [<ffffffff81477fcf>] page_fault+0x1f/0x30
> [ 84.630013] [<ffffffff810b31f8>] generic_file_aio_read+0x528/0x740
> [ 84.630017] [<ffffffff810f5461>] do_sync_read+0xd1/0x110
> [ 84.630021] [<ffffffff810f5c36>] vfs_read+0xc6/0x160
> [ 84.630023] [<ffffffff810f60b0>] sys_read+0x50/0x90
> [ 84.630025] [<ffffffff8147f4e9>] sysenter_dispatch+0x7/0x27
> [ 84.630030] [<ffffffffffffffff>] 0xffffffffffffffff
>
> ffffffff810f0e6d: 0f 84 b5 00 00 00 je ffffffff810f0f28 <kmem_cache_alloc+0xf8>
> ffffffff810f0e73: 49 63 44 24 20 movslq 0x20(%r12),%rax
> ffffffff810f0e78: 48 8d 4a 40 lea 0x40(%rdx),%rcx
> ffffffff810f0e7c: 49 8b 34 24 mov (%r12),%rsi
> >>ffffffff810f0e80: 49 8b 5c 05 00 mov 0x0(%r13,%rax,1),%rbx
> ffffffff810f0e85: 4c 89 e8 mov %r13,%rax
> ffffffff810f0e88: e8 83 19 0e 00 callq ffffffff811d2810 <this_cpu_cmpxchg16b_emu>
> ffffffff810f0e8d: 0f 1f 40 00 nopl 0x0(%rax)
> ffffffff810f0e91: 84 c0 test %al,%al
> ffffffff810f0e93: 74 c1 je ffffffff810f0e56 <kmem_cache_alloc+0x26>
>
>
> So this is definitely the access to object->next that triggers the fault.
>
> Problem is : my (2nd) patch only reduces the window of occurrence of the bug, since we can
> be preempted/interrupted between the kmemcheck_mark_initialized() and access to object->next :
>
> The preempt/irq could allocate the object and free it again.
>
> So KMEMCHECK would require us to block IRQ to be safe.
>
> Then, just disable SLUB_CMPXCHG_DOUBLE if KMEMCHECK is defined, as I did in my first patch.
>
> Christoph, please reconsider first version of patch (disabling SLUB_CMPXCHG_DOUBLE if
> either KMEMCHECK or DEBUG_PAGEALLOC is defined)
Or this alternate one : keep cmpxchg_double() stuff but block IRQ in
slab_alloc() if KMEMCHECK or DEBUG_PAGEALLOC
I tested it and got no kmemcheck splat
Thanks
[PATCH] slub: block IRQ in slab_alloc() if KMEMCHECK or DEBUG_PAGEALLOC
Christian Casteyde reported a KMEMCHECK splat in slub code.
Problem is now we are lockless and allow IRQ in slab_alloc, the object
we manipulate from freelist can be allocated and freed right before we
try to read object->next
Same problem can happen with DEBUG_PAGEALLOC
Reported-by: Christian Casteyde <casteyde.christian@...e.fr>
Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Pekka Enberg <penberg@...helsinki.fi>
Cc: Vegard Nossum <vegardno@....uio.no>
Cc: Christoph Lameter <cl@...ux.com>
---
mm/slub.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 94d2a33..9c34a2b 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1881,7 +1881,13 @@ debug:
* If not then __slab_alloc is called for slow processing.
*
* Otherwise we can simply pick the next object from the lockless free list.
+ * Since we access object->next, we must disable irqs if KMEMCHECK or
+ * DEBUG_PAGEALLOC is defined, since object could be allocated and freed
+ * under us.
*/
+#if !defined(CONFIG_CMPXCHG_LOCAL) || defined(CONFIG_KMEMCHECK) || defined(CONFIG_DEBUG_PAGEALLOC)
+#define MASK_IRQ_IN_SLAB_ALLOC
+#endif
static __always_inline void *slab_alloc(struct kmem_cache *s,
gfp_t gfpflags, int node, unsigned long addr)
{
@@ -1889,16 +1895,18 @@ static __always_inline void *slab_alloc(struct kmem_cache *s,
struct kmem_cache_cpu *c;
#ifdef CONFIG_CMPXCHG_LOCAL
unsigned long tid;
-#else
+#endif
+#ifdef MASK_IRQ_IN_SLAB_ALLOC
unsigned long flags;
#endif
if (slab_pre_alloc_hook(s, gfpflags))
return NULL;
-#ifndef CONFIG_CMPXCHG_LOCAL
+#ifdef MASK_IRQ_IN_SLAB_ALLOC
local_irq_save(flags);
-#else
+#endif
+#ifdef CONFIG_CMPXCHG_LOCAL
redo:
#endif
@@ -1954,7 +1962,7 @@ redo:
stat(s, ALLOC_FASTPATH);
}
-#ifndef CONFIG_CMPXCHG_LOCAL
+#ifdef MASK_IRQ_IN_SLAB_ALLOC
local_irq_restore(flags);
#endif
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists