From: Alexei Starovoitov An extension cannot be sleepable, and it is verified as a program that runs under the RCU read lock. But it runs in the context of the function that it replaces. For a sleepable program, or a function that is called from a bpf_wq or bpf_task_work callback, that is rcu_read_lock_trace(). The verifier lets such an extension use maps and pointers that rely on RCU. Its maps are not counted as maps with sleepable users either, so that what it got from them is freed behind a plain RCU grace period: WARNING: net/core/sock_map.c:903 at __sock_hash_lookup_elem+0xe8/0xf0 BUG: KASAN: slab-use-after-free in bpf_prog_b3bad285227d855b_ext_storage+0x245/0x297 Write of size 8 at addr ffff888108717a50 by task fr/955 The verifier knows the contexts that a global function is called in. Refuse to replace one that is called without the RCU read lock, and the main function of a sleepable program. A function that sleepable code only calls under bpf_rcu_read_lock() can still be replaced. The summarization test has replaced functions of sleepable programs by ones that do not sleep. Expect that to fail. Fixes: 1e6c62a88215 ("bpf: Introduce sleepable BPF programs") Signed-off-by: Alexei Starovoitov --- kernel/bpf/verifier.c | 20 ++++++++++++++++++ .../selftests/bpf/prog_tests/summarization.c | 21 +++++-------------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 2c812a5f1da8..d738ce6e7428 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -22151,6 +22151,21 @@ static int check_unreliable_func_args(struct bpf_verifier_log *log, struct btf * return -EINVAL; } +/* + * An extension is verified as a program that runs under the RCU read lock. Does + * the function that it is to replace? + */ +static bool func_has_rcu_read_lock(const struct bpf_prog *prog, int subprog) +{ + const struct bpf_func_info_aux *sub_aux = &prog->aux->func_info_aux[subprog]; + + if (!subprog) + return !prog->sleepable; + + return !sub_aux->called[BPF_FUNC_CTX_SLEEPABLE] && + !sub_aux->called[BPF_FUNC_CTX_NO_RCU_LOCK]; +} + int bpf_check_attach_target(struct bpf_verifier_log *log, const struct bpf_prog *prog, const struct bpf_prog *tgt_prog, @@ -22247,6 +22262,11 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, "Cannot extend a writable raw tracepoint program\n"); return -EINVAL; } + if (!func_has_rcu_read_lock(tgt_prog, subprog)) { + bpf_log(log, + "Cannot extend a function that runs without RCU read lock\n"); + return -EINVAL; + } tgt_changes_pkt_data = aux->func ? aux->func[subprog]->aux->changes_pkt_data : aux->changes_pkt_data; diff --git a/tools/testing/selftests/bpf/prog_tests/summarization.c b/tools/testing/selftests/bpf/prog_tests/summarization.c index 6951786044ca..3e4bcf071735 100644 --- a/tools/testing/selftests/bpf/prog_tests/summarization.c +++ b/tools/testing/selftests/bpf/prog_tests/summarization.c @@ -53,17 +53,6 @@ static void test_aux(const char *main_prog_name, err = summarization_freplace__load(freplace); print_verifier_log(log); - /* The might_sleep extension doesn't work yet as sleepable calls are not - * allowed, but preserve the check in case it's supported later and then - * this particular combination can be enabled. - */ - if (!strcmp("might_sleep", replacement) && err) { - ASSERT_HAS_SUBSTR(log, "sleepable helper bpf_copy_from_user#", "error log"); - ASSERT_EQ(err, -EINVAL, "err"); - test__skip(); - goto out; - } - if (expect_load) { ASSERT_OK(err, "summarization_freplace__load"); } else { @@ -86,9 +75,8 @@ static void test_aux(const char *main_prog_name, * Also test a case when main subprogram itself is replaced and is a single * subprogram in a program. * - * This holds for might_sleep programs. It is ok to replace might_sleep with - * might_sleep and with does_not_sleep, but does_not_sleep cannot be replaced - * with might_sleep. + * An extension is verified as a non-sleepable program, hence nothing that runs + * sleepable can be replaced. */ void test_summarization_freplace(void) { @@ -111,7 +99,7 @@ void test_summarization_freplace(void) }, }; const char *pkt_err = "Extension program changes packet data"; - const char *slp_err = "Extension program may sleep"; + const char *slp_err = "Cannot extend a function that runs without RCU read lock"; struct { const char *func; bool has_side_effect; @@ -136,7 +124,8 @@ void test_summarization_freplace(void) if (!test__start_subtest(buf)) continue; test_aux(mains[t][i].main, mains[t][i].to_be_replaced, replacements[t][j].func, - mains[t][i].has_side_effect || !replacements[t][j].has_side_effect, + t == 0 && + (mains[t][i].has_side_effect || !replacements[t][j].has_side_effect), replacements[t][j].err_msg); } } -- 2.55.0