| Message ID | 20241103184144.3765700-1-memxor@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Handle possible NULL trusted raw_tp arguments | expand |
On Sun, Nov 03, 2024 at 10:41:41AM -0800, Kumar Kartikeya Dwivedi wrote: > More context is available in [0], but the TLDR; is that the verifier > incorrectly assumes that any raw tracepoint argument will always be > non-NULL. This means that even when users correctly check possible NULL > arguments, the verifier can remove the NULL check due to incorrect > knowledge of the NULL-ness of the pointer. Secondly, kernel helpers or > kfuncs taking these trusted tracepoint arguments incorrectly assume that > all arguments will always be valid non-NULL. > > In this set, we mark raw_tp arguments as PTR_MAYBE_NULL on top of > PTR_TRUSTED, but special case their behavior when dereferencing them or > pointer arithmetic over them is involved. When passing trusted args to > helpers or kfuncs, raw_tp programs are permitted to pass possibly NULL > pointers in such cases. > > Any loads into such maybe NULL trusted PTR_TO_BTF_ID is promoted to a > PROBE_MEM load to handle emanating page faults. The verifier will ensure > NULL checks on such pointers are preserved and do not lead to dead code > elimination. > > This new behavior is not applied when ref_obj_id is non-zero, as those > pointers do not belong to raw_tp arguments, but instead acquired > objects. > > Since helpers and kfuncs already require attention for PTR_TO_BTF_ID > (non-trusted) pointers, we do not implement any protection for such > cases in this patch set, and leave it as future work for an upcoming > series. > > A selftest is included with this patch set to verify the new behavior, > and it crashes the kernel without the first patch. > > [0]: https://lore.kernel.org/bpf/CAADnVQLMPPavJQR6JFsi3dtaaLHB816JN4HCV_TFWohJ61D+wQ@mail.gmail.com > > Changelog: > ---------- > v1 -> v2 > v1: https://lore.kernel.org/bpf/20241101000017.3424165-1-memxor@gmail.com > > * Add patch to clean up users of gettid (Andrii) > * Avoid nested blocks in sefltest (Andrii) > * Prevent code motion optimization in selftest using barrier() > > Kumar Kartikeya Dwivedi (3): > bpf: Mark raw_tp arguments with PTR_MAYBE_NULL > selftests/bpf: Clean up open-coded gettid syscall invocations > selftests/bpf: Add tests for raw_tp null handling thanks a lot for fixing this! lgtm Reviewed-by: Jiri Olsa <jolsa@kernel.org> jirka > > include/linux/bpf.h | 6 ++ > kernel/bpf/btf.c | 5 +- > kernel/bpf/verifier.c | 75 +++++++++++++++++-- > .../selftests/bpf/benchs/bench_trigger.c | 3 +- > .../bpf/bpf_testmod/bpf_testmod-events.h | 8 ++ > .../selftests/bpf/bpf_testmod/bpf_testmod.c | 2 + > tools/testing/selftests/bpf/bpf_util.h | 9 +++ > .../bpf/map_tests/task_storage_map.c | 3 +- > .../selftests/bpf/prog_tests/bpf_cookie.c | 2 +- > .../selftests/bpf/prog_tests/bpf_iter.c | 6 +- > .../bpf/prog_tests/cgrp_local_storage.c | 10 +-- > .../selftests/bpf/prog_tests/core_reloc.c | 2 +- > .../selftests/bpf/prog_tests/linked_funcs.c | 2 +- > .../bpf/prog_tests/ns_current_pid_tgid.c | 2 +- > .../selftests/bpf/prog_tests/raw_tp_null.c | 25 +++++++ > .../selftests/bpf/prog_tests/rcu_read_lock.c | 4 +- > .../bpf/prog_tests/task_local_storage.c | 10 +-- > .../bpf/prog_tests/uprobe_multi_test.c | 2 +- > .../testing/selftests/bpf/progs/raw_tp_null.c | 32 ++++++++ > .../bpf/progs/test_tp_btf_nullable.c | 6 +- > 20 files changed, 183 insertions(+), 31 deletions(-) > create mode 100644 tools/testing/selftests/bpf/prog_tests/raw_tp_null.c > create mode 100644 tools/testing/selftests/bpf/progs/raw_tp_null.c > > > base-commit: e626a13f6fbb4697f8734333432dca577628d09a > -- > 2.43.5 >
Hello, On 03/11/24 10:41, Kumar Kartikeya Dwivedi wrote: > More context is available in [0], but the TLDR; is that the verifier > incorrectly assumes that any raw tracepoint argument will always be > non-NULL. This means that even when users correctly check possible NULL > arguments, the verifier can remove the NULL check due to incorrect > knowledge of the NULL-ness of the pointer. Secondly, kernel helpers or > kfuncs taking these trusted tracepoint arguments incorrectly assume that > all arguments will always be valid non-NULL. > > In this set, we mark raw_tp arguments as PTR_MAYBE_NULL on top of > PTR_TRUSTED, but special case their behavior when dereferencing them or > pointer arithmetic over them is involved. When passing trusted args to > helpers or kfuncs, raw_tp programs are permitted to pass possibly NULL > pointers in such cases. > > Any loads into such maybe NULL trusted PTR_TO_BTF_ID is promoted to a > PROBE_MEM load to handle emanating page faults. The verifier will ensure > NULL checks on such pointers are preserved and do not lead to dead code > elimination. > > This new behavior is not applied when ref_obj_id is non-zero, as those > pointers do not belong to raw_tp arguments, but instead acquired > objects. > > Since helpers and kfuncs already require attention for PTR_TO_BTF_ID > (non-trusted) pointers, we do not implement any protection for such > cases in this patch set, and leave it as future work for an upcoming > series. > > A selftest is included with this patch set to verify the new behavior, > and it crashes the kernel without the first patch. > > [0]: https://lore.kernel.org/bpf/CAADnVQLMPPavJQR6JFsi3dtaaLHB816JN4HCV_TFWohJ61D+wQ@mail.gmail.com This indeed cures the issue for me! Thanks a lot for fixing it. Tested-by: Juri Lelli <juri.lelli@redhat.com> Best, Juri