Hi,
This series adds missing memory access tags (MEM_RDONLY or MEM_WRITE) to several bpf helper function prototypes that use ARG_PTR_TO_MEM but lack the correct type annotation.
Missing memory access tags in helper prototypes can lead to critical correctness issues when the verifier tries to perform code optimization. After commit 37cce22dbd51 ("bpf: verifier: Refactor helper access type tracking"), the verifier relies on the memory access tags, rather than treating all arguments in helper functions as potentially modifying the pointed-to memory.
We have already seen several reports regarding this:
- commit ac44dcc788b9 ("bpf: Fix verifier assumptions of bpf_d_path's output buffer") adds MEM_WRITE to bpf_d_path; - commit 2eb7648558a7 ("bpf: Specify access type of bpf_sysctl_get_name args") adds MEM_WRITE to bpf_sysctl_get_name.
This series looks through all prototypes in the kernel and completes the tags. In addition, this series also adds selftests for some of these functions.
I marked the series as RFC since the introduced selftests are fragile and ad hoc (similar to the previously added selftests). The original goal of these tests is to reproduce the case where the verifier wrongly optimizes reads after the helper function is called. However, triggering the error often requires carefully designed code patterns. For example, I had to explicitly use "if (xx != 0)" in my attached tests, because using memcmp will not reproduce the issue. This makes the reproduction heavily dependent on the verifier's internal optimization logic and clutters the selftests with specific, unnatural patterns.
Some cases are also hard to trigger by selftests. For example, I couldn't find a triggering pattern for bpf_read_branch_records, since the execution of program seems to be messed up by wrong tags. For bpf_skb_fib_lookup, I also failed to reproduce it because the argument needs content on entry, but the verifier seems to only enable this optimization for fully empty buffers.
Since adding selftests does not help with existing issues or prevent future occurrences of similar problems, I believe one way to resolve it is to statically restrict ARG_PTR_TO_MEM from appearing without memory access tags. Using ARG_PTR_TO_MEM alone without tags is nonsensical because:
- If the helper does not change the argument, missing MEM_RDONLY causes the verifier to incorrectly reject a read-only buffer. - If the helper does change the argument, missing MEM_WRITE causes the verifier to incorrectly assume the memory is unchanged, leading to potential errors.
I am still wondering, if we agree on the above, how should we enforce this restriction? Should we let ARG_PTR_TO_MEM imply MEM_WRITE semantics by default, and change ARG_PTR_TO_MEM | MEM_RDONLY to ARG_CONST_PTR_TO_MEM? Or should we add a check in the verifier to ensure ARG_PTR_TO_MEM always comes with an access tag (though this seems to only catch errors at runtime/testing)?
Any insights and comments are welcome. If the individual fix patches for the prototypes look correct, I would also really appreciate it if they could be merged ahead of the discussion.
Thanks,
Zesen Liu
Signed-off-by: Zesen Liu ftyghome@gmail.com --- Zesen Liu (2): bpf: Fix memory access tags in helper prototypes selftests/bpf: add regression tests for snprintf and get_stack helpers
kernel/bpf/helpers.c | 2 +- kernel/trace/bpf_trace.c | 6 +++--- net/core/filter.c | 8 ++++---- tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c | 15 +++++++++++++-- tools/testing/selftests/bpf/prog_tests/snprintf.c | 6 ++++++ tools/testing/selftests/bpf/prog_tests/snprintf_btf.c | 3 +++ tools/testing/selftests/bpf/progs/netif_receive_skb.c | 13 ++++++++++++- tools/testing/selftests/bpf/progs/test_get_stack_rawtp.c | 11 ++++++++++- tools/testing/selftests/bpf/progs/test_snprintf.c | 12 ++++++++++++ 9 files changed, 64 insertions(+), 12 deletions(-) --- base-commit: 22cc16c04b7893d8fc22810599f49a305d600b9e change-id: 20251220-helper_proto-fb6e64182467
Best regards,