| From foo@baz Fri Dec 11 11:39:13 EST 2015 |
| From: Daniel Borkmann <daniel@iogearbox.net> |
| Date: Mon, 30 Nov 2015 13:02:56 +0100 |
| Subject: bpf, array: fix heap out-of-bounds access when updating elements |
| |
| From: Daniel Borkmann <daniel@iogearbox.net> |
| |
| [ Upstream commit fbca9d2d35c6ef1b323fae75cc9545005ba25097 ] |
| |
| During own review but also reported by Dmitry's syzkaller [1] it has been |
| noticed that we trigger a heap out-of-bounds access on eBPF array maps |
| when updating elements. This happens with each map whose map->value_size |
| (specified during map creation time) is not multiple of 8 bytes. |
| |
| In array_map_alloc(), elem_size is round_up(attr->value_size, 8) and |
| used to align array map slots for faster access. However, in function |
| array_map_update_elem(), we update the element as ... |
| |
| memcpy(array->value + array->elem_size * index, value, array->elem_size); |
| |
| ... where we access 'value' out-of-bounds, since it was allocated from |
| map_update_elem() from syscall side as kmalloc(map->value_size, GFP_USER) |
| and later on copied through copy_from_user(value, uvalue, map->value_size). |
| Thus, up to 7 bytes, we can access out-of-bounds. |
| |
| Same could happen from within an eBPF program, where in worst case we |
| access beyond an eBPF program's designated stack. |
| |
| Since 1be7f75d1668 ("bpf: enable non-root eBPF programs") didn't hit an |
| official release yet, it only affects priviledged users. |
| |
| In case of array_map_lookup_elem(), the verifier prevents eBPF programs |
| from accessing beyond map->value_size through check_map_access(). Also |
| from syscall side map_lookup_elem() only copies map->value_size back to |
| user, so nothing could leak. |
| |
| [1] http://github.com/google/syzkaller |
| |
| Fixes: 28fbcfa08d8e ("bpf: add array type of eBPF maps") |
| Reported-by: Dmitry Vyukov <dvyukov@google.com> |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Acked-by: Alexei Starovoitov <ast@kernel.org> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| kernel/bpf/arraymap.c | 2 +- |
| 1 file changed, 1 insertion(+), 1 deletion(-) |
| |
| --- a/kernel/bpf/arraymap.c |
| +++ b/kernel/bpf/arraymap.c |
| @@ -109,7 +109,7 @@ static int array_map_update_elem(struct |
| /* 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; |
| } |
| |