Skip to content
Commit 58990d1f authored by Daniel Borkmann's avatar Daniel Borkmann Committed by Alexei Starovoitov
Browse files

bpf: reject passing modified ctx to helper functions

As commit 28e33f9d ("bpf: disallow arithmetic operations on
context pointer") already describes, f1174f77 ("bpf/verifier:
rework value tracking") removed the specific white-listed cases
we had previously where we would allow for pointer arithmetic in
order to further generalize it, and allow e.g. context access via
modified registers. While the dereferencing of modified context
pointers had been forbidden through 28e33f9d, syzkaller did
recently manage to trigger several KASAN splats for slab out of
bounds access and use after frees by simply passing a modified
context pointer to a helper function which would then do the bad
access since verifier allowed it in adjust_ptr_min_max_vals().

Rejecting arithmetic on ctx pointer in adjust_ptr_min_max_vals()
generally could break existing programs as there's a valid use
case in tracing in combination with passing the ctx to helpers as
bpf_probe_read(), where the register then becomes unknown at
verification time due to adding a non-constant offset to it. An
access sequence may look like the following:

  offset = args->filename;  /* field __data_loc filename */
  bpf_probe_read(&dst, len, (char *)args + offset); // args is ctx

There are two options: i) we could special case the ctx and as
soon as we add a constant or bounded offset to it (hence ctx type
wouldn't change) we could turn the ctx into an unknown scalar, or
ii) we generalize the sanity test for ctx member access into a
small helper and assert it on the ctx register that was passed
as a function argument. Fwiw, latter is more obvious and less
complex at the same time, and one case that may potentially be
legitimate in future for ctx member access at least would be for
ctx to carry a const offset. Therefore, fix follows approach
from ii) and adds test cases to BPF kselftests.

Fixes: f1174f77

 ("bpf/verifier: rework value tracking")
Reported-by: default avatar <syzbot+3d0b2441dbb71751615e@syzkaller.appspotmail.com>
Reported-by: default avatar <syzbot+c8504affd4fdd0c1b626@syzkaller.appspotmail.com>
Reported-by: default avatar <syzbot+e5190cb881d8660fb1a3@syzkaller.appspotmail.com>
Reported-by: default avatar <syzbot+efae31b384d5badbd620@syzkaller.appspotmail.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
Acked-by: default avatarYonghong Song <yhs@fb.com>
Acked-by: default avatarEdward Cree <ecree@solarflare.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 1c8c5a9d
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment