Re: UBSAN: array-index-out-of-bounds in kernel/bpf/arraymap.c:177
From: Yonghong Song
Date: Mon May 18 2020 - 21:37:09 EST
On 5/18/20 6:30 PM, Andrii Nakryiko wrote:
On Mon, May 18, 2020 at 6:00 PM Yonghong Song <yhs@xxxxxx> wrote:
On 5/18/20 5:25 PM, Andrii Nakryiko wrote:
On Mon, May 18, 2020 at 5:09 PM Qian Cai <cai@xxxxxx> wrote:
On Mon, May 18, 2020 at 7:55 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
On Sun, May 17, 2020 at 7:45 PM Qian Cai <cai@xxxxxx> wrote:
With Clang 9.0.1,
return array->value + array->elem_size * (index & array->index_mask);
but array->value is,
char value[0] __aligned(8);
This, and ptrs and pptrs, should be flexible arrays. But they are in a
union, and unions don't support flexible arrays. Putting each of them
into anonymous struct field also doesn't work:
/data/users/andriin/linux/include/linux/bpf.h:820:18: error: flexible
array member in a struct with no named members
struct { void *ptrs[] __aligned(8); };
So it probably has to stay this way. Is there a way to silence UBSAN
for this particular case?
I am not aware of any way to disable a particular function in UBSAN
except for the whole file in kernel/bpf/Makefile,
UBSAN_SANITIZE_arraymap.o := n
If there is no better way to do it, I'll send a patch for it.
That's probably going to be too drastic, we still would want to
validate the rest of arraymap.c code, probably. Not sure, maybe
someone else has better ideas.
Maybe something like below?
struct bpf_array {
struct bpf_map map;
u32 elem_size;
u32 index_mask;
struct bpf_array_aux *aux;
union {
char value;
void *ptrs;
void __percpu *pptrs;
} u[] __aligned(8);
That will require wider code changes, and would look quite unnatural:
array->u[whatever].pptrs
instead of current
array->pptrs[whatever]
Right. There will be a tradeoff between to make it work vs.
some code ugliness :-). BTW, I don't have a strong preference
on how to solve this particular issue.
};