[bpf-next,v3,0/3] Handle possible NULL trusted raw_tp arguments

Message

Kumar Kartikeya Dwivedi Nov. 4, 2024, 5:19 p.m. UTC

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/[e-mail]

Changelog:
----------
v2 -> v3
v2: https://lore.kernel.org/bpf/[e-mail]

 * Fix lenient check around check_ptr_to_btf_access allowing any
   PTR_TO_BTF_ID with PTR_MAYBE_NULL to be deref'd.
 * Add Juri and Jiri's Tested-by, Reviewed-by resp.

v1 -> v2
v1: https://lore.kernel.org/bpf/[e-mail]

 * 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

 include/linux/bpf.h                           |  6 ++
 kernel/bpf/btf.c                              |  5 +-
 kernel/bpf/verifier.c                         | 79 +++++++++++++++++--
 .../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, 187 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: f2daa5a577e95f4be4e9ffae17b5bbf1ffe7a852

Comments

[e-mail] Nov. 4, 2024, 7:50 p.m. UTC | #1

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Mon,  4 Nov 2024 09:19:56 -0800 you 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.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3,1/3] bpf: Mark raw_tp arguments with PTR_MAYBE_NULL
    https://git.kernel.org/bpf/bpf-next/c/cb4158ce8ec8
  - [bpf-next,v3,2/3] selftests/bpf: Clean up open-coded gettid syscall invocations
    https://git.kernel.org/bpf/bpf-next/c/0e2fb011a0ba
  - [bpf-next,v3,3/3] selftests/bpf: Add tests for raw_tp null handling
    https://git.kernel.org/bpf/bpf-next/c/d798ce3f4cab

You are awesome, thank you!