Re: [PATCH bpf-next] bpf: avoid overflows involving hash elem_size

From: Roman Gushchin <guro@fb.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	"David S . Miller" <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	Eric Dumazet <edumazet@google.com>, bpf <bpf@vger.kernel.org>,
	syzbot <syzkaller@googlegroups.com>
Subject: Re: [PATCH bpf-next] bpf: avoid overflows involving hash elem_size
Date: Mon, 7 Dec 2020 12:52:27 -0800	[thread overview]
Message-ID: <20201207205227.GD2238414@carbon.dhcp.thefacebook.com> (raw)
In-Reply-To: <20201207182821.3940306-1-eric.dumazet@gmail.com>

On Mon, Dec 07, 2020 at 10:28:21AM -0800, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Use of bpf_map_charge_init() was making sure hash tables would not use more
> than 4GB of memory.
> 
> Since the implicit check disappeared, we have to be more careful
> about overflows, to support big hash tables.
> 
> syzbot triggers a panic using :
> 
> bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_LRU_HASH, key_size=16384, value_size=8,
>                      max_entries=262200, map_flags=0, inner_map_fd=-1, map_name="",
>                      map_ifindex=0, btf_fd=-1, btf_key_type_id=0, btf_value_type_id=0,
>                      btf_vmlinux_value_type_id=0}, 64) = ...
> 
> BUG: KASAN: vmalloc-out-of-bounds in bpf_percpu_lru_populate kernel/bpf/bpf_lru_list.c:594 [inline]
> BUG: KASAN: vmalloc-out-of-bounds in bpf_lru_populate+0x4ef/0x5e0 kernel/bpf/bpf_lru_list.c:611
> Write of size 2 at addr ffffc90017e4a020 by task syz-executor.5/19786
> 
> CPU: 0 PID: 19786 Comm: syz-executor.5 Not tainted 5.10.0-rc3-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x107/0x163 lib/dump_stack.c:118
>  print_address_description.constprop.0.cold+0x5/0x4c8 mm/kasan/report.c:385
>  __kasan_report mm/kasan/report.c:545 [inline]
>  kasan_report.cold+0x1f/0x37 mm/kasan/report.c:562
>  bpf_percpu_lru_populate kernel/bpf/bpf_lru_list.c:594 [inline]
>  bpf_lru_populate+0x4ef/0x5e0 kernel/bpf/bpf_lru_list.c:611
>  prealloc_init kernel/bpf/hashtab.c:319 [inline]
>  htab_map_alloc+0xf6e/0x1230 kernel/bpf/hashtab.c:507
>  find_and_alloc_map kernel/bpf/syscall.c:123 [inline]
>  map_create kernel/bpf/syscall.c:829 [inline]
>  __do_sys_bpf+0xa81/0x5170 kernel/bpf/syscall.c:4336
>  do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
>  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x45deb9
> Code: 0d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 db b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007fd93fbc0c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> RAX: ffffffffffffffda RBX: 0000000000001a40 RCX: 000000000045deb9
> RDX: 0000000000000040 RSI: 0000000020000280 RDI: 0000000000000000
> RBP: 000000000119bf60 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 000000000119bf2c
> R13: 00007ffc08a7be8f R14: 00007fd93fbc19c0 R15: 000000000119bf2c
> 
> Fixes: 755e5d55367a ("bpf: Eliminate rlimit-based memory accounting for hashtab maps")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Roman Gushchin <guro@fb.com>
> ---

Good catch, thank you!

Acked-by: Roman Gushchin <guro@fb.com>

It looks like there are a couple more places like this, I'll check them and send
a separate patch.

Thanks!