[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <565C2958.4040402@iogearbox.net>
Date: Mon, 30 Nov 2015 11:47:52 +0100
From: Daniel Borkmann <daniel@...earbox.net>
To: Dmitry Vyukov <dvyukov@...gle.com>,
Alexei Starovoitov <ast@...nel.org>,
netdev <netdev@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
CC: syzkaller <syzkaller@...glegroups.com>,
Kostya Serebryany <kcc@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Eric Dumazet <edumazet@...gle.com>,
Sasha Levin <sasha.levin@...cle.com>
Subject: Re: heap out-of-bounds access in array_map_update_elem
On 11/30/2015 11:42 AM, Dmitry Vyukov wrote:
> Hello,
>
>
> The following program causes heap-of-bounds access in array_map_update_elem:
>
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <syscall.h>
> #include <string.h>
> #include <stdint.h>
>
> #define SYS_bpf 321
>
> int main()
> {
> long r0 = syscall(SYS_mmap, 0x20000000ul, 0x10000ul, 0x3ul,
> 0x32ul, 0xfffffffffffffffful, 0x0ul);
> *(uint32_t*)0x20000d9b = 0x2;
> *(uint32_t*)0x20000d9f = 0x4;
> *(uint32_t*)0x20000da3 = 0x7;
> *(uint32_t*)0x20000da7 = 0x9;
> long r5 = syscall(SYS_bpf, 0x0ul, 0x20000d9bul, 0x10ul, 0, 0, 0);
> *(uint32_t*)0x20001fe0 = r5;
> *(uint32_t*)0x20001fe4 = 0x0;
> *(uint64_t*)0x20001fe8 = 0x20001000;
> *(uint64_t*)0x20001ff0 = 0x20000f42;
> *(uint64_t*)0x20001ff8 = 0x0;
> long r11 = syscall(SYS_bpf, 0x2ul, 0x20001fe0ul, 0x20ul, 0, 0, 0);
> *(uint32_t*)0x20001fe0 = r5;
> *(uint32_t*)0x20001fe4 = 0x0;
> *(uint64_t*)0x20001fe8 = 0x20001000;
> *(uint64_t*)0x20001ff0 = 0x20000f42;
> *(uint64_t*)0x20001ff8 = 0x0;
> long r17 = syscall(SYS_bpf, 0x2ul, 0x20001fe0ul, 0x20ul, 0, 0, 0);
> return 0;
> }
>
>
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in memcpy+0x1d/0x40 at addr ffff88003a6bd110
> Read of size 8 by task a.out/6260
> =============================================================================
> BUG kmalloc-8 (Tainted: G B ): kasan: bad access detected
> -----------------------------------------------------------------------------
>
> INFO: Allocated in SyS_bpf+0xfd4/0x1a20 age=4 cpu=0 pid=6260
> [< none >] __slab_alloc+0x235/0x570 mm/slub.c:2399
> [< inline >] slab_alloc_node mm/slub.c:2467
> [< inline >] slab_alloc mm/slub.c:2509
> [< none >] __kmalloc+0x1f7/0x260 mm/slub.c:3414
> [< inline >] kmalloc include/linux/slab.h:445
> [< inline >] map_update_elem kernel/bpf/syscall.c:288
> [< inline >] SYSC_bpf kernel/bpf/syscall.c:744
> [< none >] SyS_bpf+0xfd4/0x1a20 kernel/bpf/syscall.c:695
> [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
>
> INFO: Slab 0xffffea0000e9af00 objects=24 used=14 fp=0xffff88003a6bd3b0
> flags=0x1fffc0000004080
> INFO: Object 0xffff88003a6bd110 @offset=4368 fp=0xffff88003a6bc930
> CPU: 0 PID: 6260 Comm: a.out Tainted: G B 4.4.0-rc1+ #129
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> 00000000ffffffff ffff880033347ce0 ffffffff82745526 ffff88003e804280
> ffff88003a6bd110 ffff88003a6bc000 ffff880033347d10 ffffffff816229c4
> ffff88003e804280 ffffea0000e9af00 ffff88003a6bd110 0000000000000000
>
> Call Trace:
> [<ffffffff8162a294>] __asan_loadN+0x124/0x1a0 mm/kasan/kasan.c:510
> [<ffffffff8162a81d>] memcpy+0x1d/0x40 mm/kasan/kasan.c:296
> [<ffffffff814fd66a>] array_map_update_elem+0xca/0x110 kernel/bpf/arraymap.c:108
> [< inline >] map_update_elem kernel/bpf/syscall.c:300
> [< inline >] SYSC_bpf kernel/bpf/syscall.c:744
> [<ffffffff814efa89>] SyS_bpf+0x1509/0x1a20 kernel/bpf/syscall.c:695
> [<ffffffff85988836>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> ==================================================================
>
>
> On commit 90b55590c43258a157a2a143748455dcc50fbb53 (Nov 20).
>
>
> The OOB accesses happens here:
>
> static int array_map_update_elem(struct bpf_map *map, void *key, void
> *value, u64 map_flags)
> {
> ....
> memcpy(array->value + array->elem_size * index, value, array->elem_size);
>
Hehe, funny, I was just working on a fix already, just noticed this during review
on the other patch. We need something like this, will submit it properly in some
minutes:
From 5e5ac319463d27a744367eae183cda9c0759a0a1 Mon Sep 17 00:00:00 2001
Message-Id: <5e5ac319463d27a744367eae183cda9c0759a0a1.1448880311.git.daniel@...earbox.net>
From: Daniel Borkmann <daniel@...earbox.net>
Date: Mon, 30 Nov 2015 11:29:55 +0100
Subject: [PATCH net] bpf, array map: fix buffer overflow and memory leakage
Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
---
kernel/bpf/arraymap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 3f4c99e..4c67ce3 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -105,7 +105,7 @@ static int array_map_update_elem(struct bpf_map *map, void *key, void *value,
/* all elements already exist */
return -EEXIST;
- memcpy(array->value + array->elem_size * index, value, array->elem_size);
+ memcpy(array->value + array->elem_size * index, value, map->value_size);
return 0;
}
--
1.9.3
> Memory allocated for value is of size array->value_size, which can be
> up to 7 bytes less than array->elem_size. Most likely it cannot lead
> to GPF when called from syscall with current slab implementation. I
> don't know whether this OOB can cause any issues when called from a
> BPF program.
> It also copies uninit garbage into the map. I don't see how that
> garbage can be fetched later either by syscall or by a BPF program.
> But still I think it is better to copy array->elem_size and leave the
> trailer zero initialized just to be on the safer side.
>
> Thanks
> --
> 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
>
--
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