A few more fixes for bad PTR_TO_BTF_ID reg->off being accepted in places, that can lead to the kernel crashing. Noticed while making sure my own series for BTF ID pointer in map won't allow stores for pointers with incorrect offsets. I include one example where d_path can crash even if you NULL check PTR_TO_BTF_ID, and one example of how missing NULL check in helper taking PTR_TO_BTF_ID (like bpf_sock_from_file) is a real problem, see the selftest patch. The &f->f_path becomes NULL + offset in case f is NULL, circumventing NULL checks in existing helpers. The only thing needed to trigger this finding an object that embeds the object of interest, and then somehow obtaining a NULL PTR_TO_BTF_ID to it (not hard, esp. due to exception handler for PROBE_MEM loads writing 0 to destination register). However, for the case of patch 2, it is allowed in my series since the next load of the bad pointer stored using: struct file *f = ...; // some pointer walking returning NULL pointer map_val->ptr = &f->f_path; // ptr being struct path * ... would be marked as PTR_UNTRUSTED, so it won't be allowed to be passed into the kernel, and hence can be permitted. In referenced case, the PTR_TO_BTF_ID should not be NULL anyway. kptr_get style helper takes PTR_TO_MAP_VALUE in referenced ptr case only, so the load either yields NULL or RCU protected pointer. Tests for patch 1 depend on fixup_kfunc_btf_id in test_verifier, hence will be sent after merge window opens, some other changes after bpf tree merges into bpf-next, but all pending ones can be seen here [0]. Tests for patch 2 are included, and try to trigger crash without the fix, but it's not 100% reliable. We may need special testing helpers or kfuncs to make it thorough, but wanted to wait before getting feedback. Issue fixed by patch 2 is a bit more broader in scope, and would require proper discussion (before being applied) on the correct way forward, as it is technically backwards incompatible change, but hopefully never breaks real programs, only malicious or already incorrect ones. Also, please suggest the right "Fixes" tag for patch 2. As for patch 3 (selftest), please suggest a better way to get a certain type of PTR_TO_BTF_ID which can be NULL or NULL+offset. Can we add kfuncs for testing that return such pointers and make them available to e.g. TC progs, if the fix in patch 2 is acceptable? [0]: https://github.com/kkdwivedi/linux/commits/fixes-bpf-next Kumar Kartikeya Dwivedi (5): bpf: Fix kfunc register offset check for PTR_TO_BTF_ID bpf: Restrict PTR_TO_BTF_ID offset to PAGE_SIZE when calling helpers bpf: Use bpf_ptr_is_invalid for all helpers taking PTR_TO_BTF_ID selftests/bpf: Add selftest for PTR_TO_BTF_ID NULL + off case selftests/bpf: Adjust verifier selftest for updated message include/linux/bpf.h | 19 ++++ include/linux/bpf_verifier.h | 3 + kernel/bpf/bpf_inode_storage.c | 4 +- kernel/bpf/bpf_lsm.c | 4 +- kernel/bpf/bpf_task_storage.c | 4 +- kernel/bpf/btf.c | 24 ++++- kernel/bpf/stackmap.c | 3 + kernel/bpf/task_iter.c | 2 +- kernel/bpf/verifier.c | 99 +++++++++++++------ kernel/trace/bpf_trace.c | 12 +++ net/core/bpf_sk_storage.c | 9 +- net/core/filter.c | 52 ++++++---- net/ipv4/bpf_tcp_ca.c | 4 +- .../selftests/bpf/prog_tests/d_path_crash.c | 19 ++++ .../selftests/bpf/progs/d_path_crash.c | 26 +++++ .../selftests/bpf/verifier/bounds_deduction.c | 2 +- tools/testing/selftests/bpf/verifier/ctx.c | 8 +- 17 files changed, 226 insertions(+), 68 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path_crash.c create mode 100644 tools/testing/selftests/bpf/progs/d_path_crash.c -- 2.35.1
