Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'fix-resource-leak-checks-for-tail-calls'

Kumar Kartikeya Dwivedi says:

====================
Fix resource leak checks for tail calls

This set contains a fix for detecting unreleased RCU read locks or
unfinished preempt_disable sections when performing a tail call. Spin
locks are prevented by accident since they don't allow any function
calls, including tail calls (modelled as call instruction to a helper),
so we ensure they are checked as well, in preparation for relaxing
function call restricton for critical sections in the future.

Then, in the second patch, all the checks for reference leaks and locks
are unified into a single function that can be called from different
places. This unification patch is kept separate and placed after the fix
to allow independent backport of the fix to older kernels without a
depdendency on the clean up.

Naturally, this creates a divergence in the disparate error messages,
therefore selftests that rely on the exact error strings need to be
updated to match the new verifier log message.

A selftest is included to ensure no regressions occur wrt this behavior.
====================

Link: https://lore.kernel.org/r/20241103225940.1408302-1-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

+118 -53
+34 -41
kernel/bpf/verifier.c
··· 10352 10352 return refs_lingering ? -EINVAL : 0; 10353 10353 } 10354 10354 10355 + static int check_resource_leak(struct bpf_verifier_env *env, bool exception_exit, bool check_lock, const char *prefix) 10356 + { 10357 + int err; 10358 + 10359 + if (check_lock && env->cur_state->active_lock.ptr) { 10360 + verbose(env, "%s cannot be used inside bpf_spin_lock-ed region\n", prefix); 10361 + return -EINVAL; 10362 + } 10363 + 10364 + err = check_reference_leak(env, exception_exit); 10365 + if (err) { 10366 + verbose(env, "%s would lead to reference leak\n", prefix); 10367 + return err; 10368 + } 10369 + 10370 + if (check_lock && env->cur_state->active_rcu_lock) { 10371 + verbose(env, "%s cannot be used inside bpf_rcu_read_lock-ed region\n", prefix); 10372 + return -EINVAL; 10373 + } 10374 + 10375 + if (check_lock && env->cur_state->active_preempt_lock) { 10376 + verbose(env, "%s cannot be used inside bpf_preempt_disable-ed region\n", prefix); 10377 + return -EINVAL; 10378 + } 10379 + 10380 + return 0; 10381 + } 10382 + 10355 10383 static int check_bpf_snprintf_call(struct bpf_verifier_env *env, 10356 10384 struct bpf_reg_state *regs) 10357 10385 { ··· 10648 10620 10649 10621 switch (func_id) { 10650 10622 case BPF_FUNC_tail_call: 10651 - err = check_reference_leak(env, false); 10652 - if (err) { 10653 - verbose(env, "tail_call would lead to reference leak\n"); 10623 + err = check_resource_leak(env, false, true, "tail_call"); 10624 + if (err) 10654 10625 return err; 10655 - } 10656 10626 break; 10657 10627 case BPF_FUNC_get_local_storage: 10658 10628 /* check that flags argument in get_local_storage(map, flags) is 0, ··· 15812 15786 * gen_ld_abs() may terminate the program at runtime, leading to 15813 15787 * reference leak. 15814 15788 */ 15815 - err = check_reference_leak(env, false); 15816 - if (err) { 15817 - verbose(env, "BPF_LD_[ABS|IND] cannot be mixed with socket references\n"); 15789 + err = check_resource_leak(env, false, true, "BPF_LD_[ABS|IND]"); 15790 + if (err) 15818 15791 return err; 15819 - } 15820 - 15821 - if (env->cur_state->active_lock.ptr) { 15822 - verbose(env, "BPF_LD_[ABS|IND] cannot be used inside bpf_spin_lock-ed region\n"); 15823 - return -EINVAL; 15824 - } 15825 - 15826 - if (env->cur_state->active_rcu_lock) { 15827 - verbose(env, "BPF_LD_[ABS|IND] cannot be used inside bpf_rcu_read_lock-ed region\n"); 15828 - return -EINVAL; 15829 - } 15830 - 15831 - if (env->cur_state->active_preempt_lock) { 15832 - verbose(env, "BPF_LD_[ABS|IND] cannot be used inside bpf_preempt_disable-ed region\n"); 15833 - return -EINVAL; 15834 - } 15835 15792 15836 15793 if (regs[ctx_reg].type != PTR_TO_CTX) { 15837 15794 verbose(env, ··· 18600 18591 return -EINVAL; 18601 18592 } 18602 18593 process_bpf_exit_full: 18603 - if (env->cur_state->active_lock.ptr && !env->cur_state->curframe) { 18604 - verbose(env, "bpf_spin_unlock is missing\n"); 18605 - return -EINVAL; 18606 - } 18607 - 18608 - if (env->cur_state->active_rcu_lock && !env->cur_state->curframe) { 18609 - verbose(env, "bpf_rcu_read_unlock is missing\n"); 18610 - return -EINVAL; 18611 - } 18612 - 18613 - if (env->cur_state->active_preempt_lock && !env->cur_state->curframe) { 18614 - verbose(env, "%d bpf_preempt_enable%s missing\n", 18615 - env->cur_state->active_preempt_lock, 18616 - env->cur_state->active_preempt_lock == 1 ? " is" : "(s) are"); 18617 - return -EINVAL; 18618 - } 18619 - 18620 18594 /* We must do check_reference_leak here before 18621 18595 * prepare_func_exit to handle the case when 18622 18596 * state->curframe > 0, it may be a callback 18623 18597 * function, for which reference_state must 18624 18598 * match caller reference state when it exits. 18625 18599 */ 18626 - err = check_reference_leak(env, exception_exit); 18600 + err = check_resource_leak(env, exception_exit, !env->cur_state->curframe, 18601 + "BPF_EXIT instruction"); 18627 18602 if (err) 18628 18603 return err; 18629 18604
+8
tools/testing/selftests/bpf/prog_tests/tailcalls.c
··· 7 7 #include "tailcall_bpf2bpf_hierarchy3.skel.h" 8 8 #include "tailcall_freplace.skel.h" 9 9 #include "tc_bpf2bpf.skel.h" 10 + #include "tailcall_fail.skel.h" 10 11 11 12 /* test_tailcall_1 checks basic functionality by patching multiple locations 12 13 * in a single program for a single tail call slot with nop->jmp, jmp->nop ··· 1647 1646 tc_bpf2bpf__destroy(tc_skel); 1648 1647 } 1649 1648 1649 + static void test_tailcall_failure() 1650 + { 1651 + RUN_TESTS(tailcall_fail); 1652 + } 1653 + 1650 1654 void test_tailcalls(void) 1651 1655 { 1652 1656 if (test__start_subtest("tailcall_1")) ··· 1704 1698 test_tailcall_freplace(); 1705 1699 if (test__start_subtest("tailcall_bpf2bpf_freplace")) 1706 1700 test_tailcall_bpf2bpf_freplace(); 1701 + if (test__start_subtest("tailcall_failure")) 1702 + test_tailcall_failure(); 1707 1703 }
+2 -2
tools/testing/selftests/bpf/progs/exceptions_fail.c
··· 131 131 } 132 132 133 133 SEC("?tc") 134 - __failure __msg("bpf_rcu_read_unlock is missing") 134 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_rcu_read_lock-ed region") 135 135 int reject_with_rcu_read_lock(void *ctx) 136 136 { 137 137 bpf_rcu_read_lock(); ··· 147 147 } 148 148 149 149 SEC("?tc") 150 - __failure __msg("bpf_rcu_read_unlock is missing") 150 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_rcu_read_lock-ed region") 151 151 int reject_subprog_with_rcu_read_lock(void *ctx) 152 152 { 153 153 bpf_rcu_read_lock();
+7 -7
tools/testing/selftests/bpf/progs/preempt_lock.c
··· 6 6 #include "bpf_experimental.h" 7 7 8 8 SEC("?tc") 9 - __failure __msg("1 bpf_preempt_enable is missing") 9 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") 10 10 int preempt_lock_missing_1(struct __sk_buff *ctx) 11 11 { 12 12 bpf_preempt_disable(); ··· 14 14 } 15 15 16 16 SEC("?tc") 17 - __failure __msg("2 bpf_preempt_enable(s) are missing") 17 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") 18 18 int preempt_lock_missing_2(struct __sk_buff *ctx) 19 19 { 20 20 bpf_preempt_disable(); ··· 23 23 } 24 24 25 25 SEC("?tc") 26 - __failure __msg("3 bpf_preempt_enable(s) are missing") 26 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") 27 27 int preempt_lock_missing_3(struct __sk_buff *ctx) 28 28 { 29 29 bpf_preempt_disable(); ··· 33 33 } 34 34 35 35 SEC("?tc") 36 - __failure __msg("1 bpf_preempt_enable is missing") 36 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") 37 37 int preempt_lock_missing_3_minus_2(struct __sk_buff *ctx) 38 38 { 39 39 bpf_preempt_disable(); ··· 55 55 } 56 56 57 57 SEC("?tc") 58 - __failure __msg("1 bpf_preempt_enable is missing") 58 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") 59 59 int preempt_lock_missing_1_subprog(struct __sk_buff *ctx) 60 60 { 61 61 preempt_disable(); ··· 63 63 } 64 64 65 65 SEC("?tc") 66 - __failure __msg("2 bpf_preempt_enable(s) are missing") 66 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") 67 67 int preempt_lock_missing_2_subprog(struct __sk_buff *ctx) 68 68 { 69 69 preempt_disable(); ··· 72 72 } 73 73 74 74 SEC("?tc") 75 - __failure __msg("1 bpf_preempt_enable is missing") 75 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") 76 76 int preempt_lock_missing_2_minus_1_subprog(struct __sk_buff *ctx) 77 77 { 78 78 preempt_disable();
+64
tools/testing/selftests/bpf/progs/tailcall_fail.c
··· 1 + // SPDX-License-Identifier: GPL-2.0 2 + /* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ 3 + #include <vmlinux.h> 4 + #include <bpf/bpf_tracing.h> 5 + #include <bpf/bpf_helpers.h> 6 + #include <bpf/bpf_core_read.h> 7 + 8 + #include "bpf_misc.h" 9 + #include "bpf_experimental.h" 10 + 11 + extern void bpf_rcu_read_lock(void) __ksym; 12 + extern void bpf_rcu_read_unlock(void) __ksym; 13 + 14 + #define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8))) 15 + 16 + private(A) struct bpf_spin_lock lock; 17 + 18 + struct { 19 + __uint(type, BPF_MAP_TYPE_PROG_ARRAY); 20 + __uint(max_entries, 3); 21 + __uint(key_size, sizeof(__u32)); 22 + __uint(value_size, sizeof(__u32)); 23 + } jmp_table SEC(".maps"); 24 + 25 + SEC("?tc") 26 + __failure __msg("function calls are not allowed while holding a lock") 27 + int reject_tail_call_spin_lock(struct __sk_buff *ctx) 28 + { 29 + bpf_spin_lock(&lock); 30 + bpf_tail_call_static(ctx, &jmp_table, 0); 31 + return 0; 32 + } 33 + 34 + SEC("?tc") 35 + __failure __msg("tail_call cannot be used inside bpf_rcu_read_lock-ed region") 36 + int reject_tail_call_rcu_lock(struct __sk_buff *ctx) 37 + { 38 + bpf_rcu_read_lock(); 39 + bpf_tail_call_static(ctx, &jmp_table, 0); 40 + bpf_rcu_read_unlock(); 41 + return 0; 42 + } 43 + 44 + SEC("?tc") 45 + __failure __msg("tail_call cannot be used inside bpf_preempt_disable-ed region") 46 + int reject_tail_call_preempt_lock(struct __sk_buff *ctx) 47 + { 48 + bpf_guard_preempt(); 49 + bpf_tail_call_static(ctx, &jmp_table, 0); 50 + return 0; 51 + } 52 + 53 + SEC("?tc") 54 + __failure __msg("tail_call would lead to reference leak") 55 + int reject_tail_call_ref(struct __sk_buff *ctx) 56 + { 57 + struct foo { int i; } *p; 58 + 59 + p = bpf_obj_new(typeof(*p)); 60 + bpf_tail_call_static(ctx, &jmp_table, 0); 61 + return 0; 62 + } 63 + 64 + char _license[] SEC("license") = "GPL";
+2 -2
tools/testing/selftests/bpf/progs/verifier_ref_tracking.c
··· 791 791 792 792 SEC("tc") 793 793 __description("reference tracking: forbid LD_ABS while holding reference") 794 - __failure __msg("BPF_LD_[ABS|IND] cannot be mixed with socket references") 794 + __failure __msg("BPF_LD_[ABS|IND] would lead to reference leak") 795 795 __naked void ld_abs_while_holding_reference(void) 796 796 { 797 797 asm volatile (" \ ··· 836 836 837 837 SEC("tc") 838 838 __description("reference tracking: forbid LD_IND while holding reference") 839 - __failure __msg("BPF_LD_[ABS|IND] cannot be mixed with socket references") 839 + __failure __msg("BPF_LD_[ABS|IND] would lead to reference leak") 840 840 __naked void ld_ind_while_holding_reference(void) 841 841 { 842 842 asm volatile (" \
+1 -1
tools/testing/selftests/bpf/progs/verifier_spin_lock.c
··· 187 187 188 188 SEC("cgroup/skb") 189 189 __description("spin_lock: test6 missing unlock") 190 - __failure __msg("unlock is missing") 190 + __failure __msg("BPF_EXIT instruction cannot be used inside bpf_spin_lock-ed region") 191 191 __failure_unpriv __msg_unpriv("") 192 192 __naked void spin_lock_test6_missing_unlock(void) 193 193 {