From patchwork Fri Dec 15 01:13:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493870 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0a-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E0AAE7E4 for ; Fri, 15 Dec 2023 01:13:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0089730.ppops.net [127.0.0.1]) by m0089730.ppops.net (8.17.1.19/8.17.1.19) with ESMTP id 3BEJFRNG007925 for ; Thu, 14 Dec 2023 17:13:43 -0800 Received: from mail.thefacebook.com ([163.114.132.120]) by m0089730.ppops.net (PPS) with ESMTPS id 3uynkksh5g-4 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:13:43 -0800 Received: from twshared21997.42.prn1.facebook.com (2620:10d:c085:208::11) by mail.thefacebook.com (2620:10d:c085:11d::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:13:39 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id 6104B3D2CCEC0; Thu, 14 Dec 2023 17:13:37 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 01/10] bpf: abstract away global subprog arg preparation logic from reg state setup Date: Thu, 14 Dec 2023 17:13:25 -0800 Message-ID: <20231215011334.2307144-2-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: x9DhT1kaf6bc2J-cmyfxtL-fRxaylJ3a X-Proofpoint-GUID: x9DhT1kaf6bc2J-cmyfxtL-fRxaylJ3a X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net btf_prepare_func_args() is used to understand expectations and restrictions on global subprog arguments. But current implementation is hard to extend, as it intermixes BTF-based func prototype parsing and interpretation logic with setting up register state at subprog entry. Worse still, those registers are not completely set up inside btf_prepare_func_args(), requiring some more logic later in do_check_common(). Like calling mark_reg_unknown() and similar initialization operations. This intermixing of BTF interpretation and register state setup is problematic. First, it causes duplication of BTF parsing logic for global subprog verification (to set up initial state of global subprog) and global subprog call sites analysis (when we need to check that whatever is being passed into global subprog matches expectations), performed in btf_check_subprog_call(). Given we want to extend global func argument with tags later, this duplication is problematic. So refactor btf_prepare_func_args() to do only BTF-based func proto and args parsing, returning high-level argument "expectations" only, with no regard to specifics of register state. I.e., if it's a context argument, instead of setting register state to PTR_TO_CTX, we return ARG_PTR_TO_CTX enum for that argument as "an argument specification" for further processing inside do_check_common(). Similarly for SCALAR arguments, PTR_TO_MEM, etc. This allows to reuse btf_prepare_func_args() in following patches at global subprog call site analysis time. It also keeps register setup code consistently in one place, do_check_common(). Besides all this, we cache this argument specs information inside env->subprog_info, eliminating the need to redo these potentially expensive BTF traversals, especially if BPF program's BTF is big and/or there are lots of global subprog calls. Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- include/linux/bpf.h | 3 +-- include/linux/bpf_verifier.h | 16 ++++++++++++++ kernel/bpf/btf.c | 38 ++++++++++++++++--------------- kernel/bpf/verifier.c | 43 ++++++++++++++++++++++-------------- 4 files changed, 64 insertions(+), 36 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index c87c608a3689..12490de06dbb 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2495,8 +2495,7 @@ int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog, struct bpf_reg_state *regs); int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, struct bpf_reg_state *regs); -int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, - struct bpf_reg_state *reg, u32 *nargs); +int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog); int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *prog, struct btf *btf, const struct btf_type *t); const char *btf_find_decl_tag_value(const struct btf *btf, const struct btf_type *pt, diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index c2819a6579a5..5742e9c0a7b8 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -606,6 +606,13 @@ static inline bool bpf_verifier_log_needed(const struct bpf_verifier_log *log) #define BPF_MAX_SUBPROGS 256 +struct bpf_subprog_arg_info { + enum bpf_arg_type arg_type; + union { + u32 mem_size; + }; +}; + struct bpf_subprog_info { /* 'start' has to be the first field otherwise find_subprog() won't work */ u32 start; /* insn idx of function entry point */ @@ -617,6 +624,10 @@ struct bpf_subprog_info { bool is_cb: 1; bool is_async_cb: 1; bool is_exception_cb: 1; + bool args_cached: 1; + + u8 arg_cnt; + struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS]; }; struct bpf_verifier_env; @@ -727,6 +738,11 @@ struct bpf_verifier_env { char tmp_str_buf[TMP_STR_BUF_LEN]; }; +static inline struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env, int subprog) +{ + return &env->subprog_info[subprog]; +} + __printf(2, 0) void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt, va_list args); __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env, diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index d56433bf8aba..be2104e5f2f5 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6948,16 +6948,17 @@ int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, return err; } -/* Convert BTF of a function into bpf_reg_state if possible +/* Process BTF of a function to produce high-level expectation of function + * arguments (like ARG_PTR_TO_CTX, or ARG_PTR_TO_MEM, etc). This information + * is cached in subprog info for reuse. * Returns: * EFAULT - there is a verifier bug. Abort verification. * EINVAL - cannot convert BTF. - * 0 - Successfully converted BTF into bpf_reg_state - * (either PTR_TO_CTX or SCALAR_VALUE). + * 0 - Successfully processed BTF and constructed argument expectations. */ -int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, - struct bpf_reg_state *regs, u32 *arg_cnt) +int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) { + struct bpf_subprog_info *sub = subprog_info(env, subprog); struct bpf_verifier_log *log = &env->log; struct bpf_prog *prog = env->prog; enum bpf_prog_type prog_type = prog->type; @@ -6967,6 +6968,9 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, u32 i, nargs, btf_id; const char *tname; + if (sub->args_cached) + return 0; + if (!prog->aux->func_info || prog->aux->func_info_aux[subprog].linkage != BTF_FUNC_GLOBAL) { bpf_log(log, "Verifier bug\n"); @@ -6990,10 +6994,6 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, } tname = btf_name_by_offset(btf, t->name_off); - if (log->level & BPF_LOG_LEVEL) - bpf_log(log, "Validating %s() func#%d...\n", - tname, subprog); - if (prog->aux->func_info_aux[subprog].unreliable) { bpf_log(log, "Verifier bug in function %s()\n", tname); return -EFAULT; @@ -7013,7 +7013,6 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, tname, nargs, MAX_BPF_FUNC_REG_ARGS); return -EINVAL; } - *arg_cnt = nargs; /* check that function returns int, exception cb also requires this */ t = btf_type_by_id(btf, t->type); while (btf_type_is_modifier(t)) @@ -7028,24 +7027,24 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, * Only PTR_TO_CTX and SCALAR are supported atm. */ for (i = 0; i < nargs; i++) { - struct bpf_reg_state *reg = ®s[i + 1]; - t = btf_type_by_id(btf, args[i].type); while (btf_type_is_modifier(t)) t = btf_type_by_id(btf, t->type); if (btf_type_is_int(t) || btf_is_any_enum(t)) { - reg->type = SCALAR_VALUE; + sub->args[i].arg_type = ARG_ANYTHING; continue; } if (btf_type_is_ptr(t)) { + u32 mem_size; + if (btf_get_prog_ctx_type(log, btf, t, prog_type, i)) { - reg->type = PTR_TO_CTX; + sub->args[i].arg_type = ARG_PTR_TO_CTX; continue; } t = btf_type_skip_modifiers(btf, t->type, NULL); - ref_t = btf_resolve_size(btf, t, ®->mem_size); + ref_t = btf_resolve_size(btf, t, &mem_size); if (IS_ERR(ref_t)) { bpf_log(log, "arg#%d reference type('%s %s') size cannot be determined: %ld\n", @@ -7054,15 +7053,18 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, return -EINVAL; } - reg->type = PTR_TO_MEM | PTR_MAYBE_NULL; - reg->id = ++env->id_gen; - + sub->args[i].arg_type = ARG_PTR_TO_MEM_OR_NULL; + sub->args[i].mem_size = mem_size; continue; } bpf_log(log, "Arg#%d type %s in %s() is not supported yet.\n", i, btf_type_str(t), tname); return -EINVAL; } + + sub->arg_cnt = nargs; + sub->args_cached = true; + return 0; } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 1863826a4ac3..c0ac68d81356 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -442,11 +442,6 @@ static struct bpf_func_info_aux *subprog_aux(const struct bpf_verifier_env *env, return &env->prog->aux->func_info_aux[subprog]; } -static struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env, int subprog) -{ - return &env->subprog_info[subprog]; -} - static void mark_subprog_exc_cb(struct bpf_verifier_env *env, int subprog) { struct bpf_subprog_info *info = subprog_info(env, subprog); @@ -19908,34 +19903,50 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) regs = state->frame[state->curframe]->regs; if (subprog || env->prog->type == BPF_PROG_TYPE_EXT) { - u32 nargs; + struct bpf_subprog_info *sub = subprog_info(env, subprog); + const char *sub_name = subprog_name(env, subprog); + struct bpf_subprog_arg_info *arg; + struct bpf_reg_state *reg; - ret = btf_prepare_func_args(env, subprog, regs, &nargs); + verbose(env, "Validating %s() func#%d...\n", sub_name, subprog); + ret = btf_prepare_func_args(env, subprog); if (ret) goto out; + if (subprog_is_exc_cb(env, subprog)) { state->frame[0]->in_exception_callback_fn = true; /* We have already ensured that the callback returns an integer, just * like all global subprogs. We need to determine it only has a single * scalar argument. */ - if (nargs != 1 || regs[BPF_REG_1].type != SCALAR_VALUE) { + if (sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_ANYTHING) { verbose(env, "exception cb only supports single integer argument\n"); ret = -EINVAL; goto out; } } - for (i = BPF_REG_1; i <= BPF_REG_5; i++) { - if (regs[i].type == PTR_TO_CTX) + for (i = BPF_REG_1; i <= sub->arg_cnt; i++) { + arg = &sub->args[i - BPF_REG_1]; + reg = ®s[i]; + + if (arg->arg_type == ARG_PTR_TO_CTX) { + reg->type = PTR_TO_CTX; mark_reg_known_zero(env, regs, i); - else if (regs[i].type == SCALAR_VALUE) + } else if (arg->arg_type == ARG_ANYTHING) { + reg->type = SCALAR_VALUE; mark_reg_unknown(env, regs, i); - else if (base_type(regs[i].type) == PTR_TO_MEM) { - const u32 mem_size = regs[i].mem_size; - + } else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) { + reg->type = PTR_TO_MEM; + if (arg->arg_type & PTR_MAYBE_NULL) + reg->type |= PTR_MAYBE_NULL; mark_reg_known_zero(env, regs, i); - regs[i].mem_size = mem_size; - regs[i].id = ++env->id_gen; + reg->mem_size = arg->mem_size; + reg->id = ++env->id_gen; + } else { + WARN_ONCE(1, "BUG: unhandled arg#%d type %d\n", + i - BPF_REG_1, arg->arg_type); + ret = -EFAULT; + goto out; } } } else { From patchwork Fri Dec 15 01:13:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493874 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1B02D7E4 for ; Fri, 15 Dec 2023 01:13:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0044012.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3BF0eIZt026727 for ; Thu, 14 Dec 2023 17:13:56 -0800 Received: from maileast.thefacebook.com ([163.114.130.16]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3v0cf8r6h7-5 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:13:56 -0800 Received: from twshared10507.42.prn1.facebook.com (2620:10d:c0a8:1b::2d) by mail.thefacebook.com (2620:10d:c0a8:83::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:13:50 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id 6F5C13D2CCED2; Thu, 14 Dec 2023 17:13:39 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 02/10] bpf: reuse btf_prepare_func_args() check for main program BTF validation Date: Thu, 14 Dec 2023 17:13:26 -0800 Message-ID: <20231215011334.2307144-3-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: sk92RkomnJyUnrfgYwAGhbYbAMvXYpL6 X-Proofpoint-GUID: sk92RkomnJyUnrfgYwAGhbYbAMvXYpL6 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Instead of btf_check_subprog_arg_match(), use btf_prepare_func_args() logic to validate "trustworthiness" of main BPF program's BTF information, if it is present. We ignored results of original BTF check anyway, often times producing confusing and ominously-sounding "reg type unsupported for arg#0 function" message, which has no apparent effect on program correctness and verification process. All the -EFAULT returning sanity checks are already performed in check_btf_info_early(), so there is zero reason to have this duplication of logic between btf_check_subprog_call() and btf_check_subprog_arg_match(). Dropping btf_check_subprog_arg_match() simplifies btf_check_func_arg_match() further removing `bool processing_call` flag. One subtle bit that was done by btf_check_subprog_arg_match() was potentially marking main program's BTF as unreliable. We do this explicitly now with a dedicated simple check, preserving the original behavior, but now based on well factored btf_prepare_func_args() logic. Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- include/linux/bpf.h | 2 - kernel/bpf/btf.c | 50 ++----------------- kernel/bpf/verifier.c | 25 +++++----- .../selftests/bpf/prog_tests/log_fixup.c | 4 +- .../selftests/bpf/progs/cgrp_kfunc_failure.c | 2 +- .../selftests/bpf/progs/task_kfunc_failure.c | 2 +- 6 files changed, 19 insertions(+), 66 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 12490de06dbb..cf80cfe9f0be 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2491,8 +2491,6 @@ int btf_distill_func_proto(struct bpf_verifier_log *log, struct btf_func_model *m); struct bpf_reg_state; -int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog, - struct bpf_reg_state *regs); int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, struct bpf_reg_state *regs); int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog); diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index be2104e5f2f5..33d9a1c73f6e 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6768,8 +6768,7 @@ int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *pr static int btf_check_func_arg_match(struct bpf_verifier_env *env, const struct btf *btf, u32 func_id, struct bpf_reg_state *regs, - bool ptr_to_mem_ok, - bool processing_call) + bool ptr_to_mem_ok) { enum bpf_prog_type prog_type = resolve_prog_type(env->prog); struct bpf_verifier_log *log = &env->log; @@ -6842,7 +6841,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, i, btf_type_str(t)); return -EINVAL; } - } else if (ptr_to_mem_ok && processing_call) { + } else if (ptr_to_mem_ok) { const struct btf_type *resolve_ret; u32 type_size; @@ -6867,55 +6866,12 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, return 0; } -/* Compare BTF of a function declaration with given bpf_reg_state. - * Returns: - * EFAULT - there is a verifier bug. Abort verification. - * EINVAL - there is a type mismatch or BTF is not available. - * 0 - BTF matches with what bpf_reg_state expects. - * Only PTR_TO_CTX and SCALAR_VALUE states are recognized. - */ -int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog, - struct bpf_reg_state *regs) -{ - struct bpf_prog *prog = env->prog; - struct btf *btf = prog->aux->btf; - bool is_global; - u32 btf_id; - int err; - - if (!prog->aux->func_info) - return -EINVAL; - - btf_id = prog->aux->func_info[subprog].type_id; - if (!btf_id) - return -EFAULT; - - if (prog->aux->func_info_aux[subprog].unreliable) - return -EINVAL; - - is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL; - err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, false); - - /* Compiler optimizations can remove arguments from static functions - * or mismatched type can be passed into a global function. - * In such cases mark the function as unreliable from BTF point of view. - */ - if (err) - prog->aux->func_info_aux[subprog].unreliable = true; - return err; -} - /* Compare BTF of a function call with given bpf_reg_state. * Returns: * EFAULT - there is a verifier bug. Abort verification. * EINVAL - there is a type mismatch or BTF is not available. * 0 - BTF matches with what bpf_reg_state expects. * Only PTR_TO_CTX and SCALAR_VALUE states are recognized. - * - * NOTE: the code is duplicated from btf_check_subprog_arg_match() - * because btf_check_func_arg_match() is still doing both. Once that - * function is split in 2, we can call from here btf_check_subprog_arg_match() - * first, and then treat the calling part in a new code path. */ int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, struct bpf_reg_state *regs) @@ -6937,7 +6893,7 @@ int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, return -EINVAL; is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL; - err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, true); + err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global); /* Compiler optimizations can remove arguments from static functions * or mismatched type can be passed into a global function. diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index c0ac68d81356..540e4336290a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -19875,6 +19875,7 @@ static void free_states(struct bpf_verifier_env *env) static int do_check_common(struct bpf_verifier_env *env, int subprog) { bool pop_log = !(env->log.level & BPF_LOG_LEVEL2); + struct bpf_subprog_info *sub = subprog_info(env, subprog); struct bpf_verifier_state *state; struct bpf_reg_state *regs; int ret, i; @@ -19901,9 +19902,9 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) state->first_insn_idx = env->subprog_info[subprog].start; state->last_insn_idx = -1; + regs = state->frame[state->curframe]->regs; if (subprog || env->prog->type == BPF_PROG_TYPE_EXT) { - struct bpf_subprog_info *sub = subprog_info(env, subprog); const char *sub_name = subprog_name(env, subprog); struct bpf_subprog_arg_info *arg; struct bpf_reg_state *reg; @@ -19950,21 +19951,19 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) } } } else { + /* if main BPF program has associated BTF info, validate that + * it's matching expected signature, and otherwise mark BTF + * info for main program as unreliable + */ + if (env->prog->aux->func_info_aux) { + ret = btf_prepare_func_args(env, 0); + if (ret || sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_PTR_TO_CTX) + env->prog->aux->func_info_aux[0].unreliable = true; + } + /* 1st arg to a function */ regs[BPF_REG_1].type = PTR_TO_CTX; mark_reg_known_zero(env, regs, BPF_REG_1); - ret = btf_check_subprog_arg_match(env, subprog, regs); - if (ret == -EFAULT) - /* unlikely verifier bug. abort. - * ret == 0 and ret < 0 are sadly acceptable for - * main() function due to backward compatibility. - * Like socket filter program may be written as: - * int bpf_prog(struct pt_regs *ctx) - * and never dereference that ctx in the program. - * 'struct pt_regs' is a type mismatch for socket - * filter that should be using 'struct __sk_buff'. - */ - goto out; } ret = do_check(env); diff --git a/tools/testing/selftests/bpf/prog_tests/log_fixup.c b/tools/testing/selftests/bpf/prog_tests/log_fixup.c index effd78b2a657..7a3fa2ff567b 100644 --- a/tools/testing/selftests/bpf/prog_tests/log_fixup.c +++ b/tools/testing/selftests/bpf/prog_tests/log_fixup.c @@ -169,9 +169,9 @@ void test_log_fixup(void) if (test__start_subtest("bad_core_relo_trunc_none")) bad_core_relo(0, TRUNC_NONE /* full buf */); if (test__start_subtest("bad_core_relo_trunc_partial")) - bad_core_relo(300, TRUNC_PARTIAL /* truncate original log a bit */); + bad_core_relo(280, TRUNC_PARTIAL /* truncate original log a bit */); if (test__start_subtest("bad_core_relo_trunc_full")) - bad_core_relo(210, TRUNC_FULL /* truncate also libbpf's message patch */); + bad_core_relo(220, TRUNC_FULL /* truncate also libbpf's message patch */); if (test__start_subtest("bad_core_relo_subprog")) bad_core_relo_subprog(); if (test__start_subtest("missing_map")) diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c index 0fa564a5cc5b..9fe9c4a4e8f6 100644 --- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c @@ -78,7 +78,7 @@ int BPF_PROG(cgrp_kfunc_acquire_fp, struct cgroup *cgrp, const char *path) } SEC("kretprobe/cgroup_destroy_locked") -__failure __msg("reg type unsupported for arg#0 function") +__failure __msg("calling kernel function bpf_cgroup_acquire is not allowed") int BPF_PROG(cgrp_kfunc_acquire_unsafe_kretprobe, struct cgroup *cgrp) { struct cgroup *acquired; diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c index dcdea3127086..ad88a3796ddf 100644 --- a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c +++ b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c @@ -248,7 +248,7 @@ int BPF_PROG(task_kfunc_from_pid_no_null_check, struct task_struct *task, u64 cl } SEC("lsm/task_free") -__failure __msg("reg type unsupported for arg#0 function") +__failure __msg("R1 must be a rcu pointer") int BPF_PROG(task_kfunc_from_lsm_task_free, struct task_struct *task) { struct task_struct *acquired; From patchwork Fri Dec 15 01:13:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493871 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0a-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B16407EA for ; Fri, 15 Dec 2023 01:13:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0089730.ppops.net [127.0.0.1]) by m0089730.ppops.net (8.17.1.19/8.17.1.19) with ESMTP id 3BEJFRNI007925 for ; Thu, 14 Dec 2023 17:13:44 -0800 Received: from mail.thefacebook.com ([163.114.132.120]) by m0089730.ppops.net (PPS) with ESMTPS id 3uynkksh5g-6 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:13:44 -0800 Received: from twshared15991.38.frc1.facebook.com (2620:10d:c085:108::8) by mail.thefacebook.com (2620:10d:c085:11d::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:13:43 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id 79B8F3D2CCEE1; Thu, 14 Dec 2023 17:13:41 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 03/10] bpf: prepare btf_prepare_func_args() for handling static subprogs Date: Thu, 14 Dec 2023 17:13:27 -0800 Message-ID: <20231215011334.2307144-4-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: UcxNjqssn8VH0nlM6v-AES1oimYxfSJ5 X-Proofpoint-GUID: UcxNjqssn8VH0nlM6v-AES1oimYxfSJ5 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Generalize btf_prepare_func_args() to support both global and static subprogs. We are going to utilize this property in the next patch, reusing btf_prepare_func_args() for subprog call logic instead of reparsing BTF information in a completely separate implementation. btf_prepare_func_args() now detects whether subprog is global or static makes slight logic adjustments for static func cases, like not failing fatally (-EFAULT) for conditions that are allowable for static subprogs. Somewhat subtle (but major!) difference is the handling of pointer arguments. Both global and static functions need to handle special context arguments (which are pointers to predefined type names), but static subprogs give up on any other pointers, falling back to marking subprog as "unreliable", disabling the use of BTF type information altogether. For global functions, though, we are assuming that such pointers to unrecognized types are just pointers to fixed-sized memory region (or error out if size cannot be established, like for `void *` pointers). This patch accommodates these small differences and sets up a stage for refactoring in the next patch, eliminating a separate BTF-based parsing logic in btf_check_func_arg_match(). Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- include/linux/bpf_verifier.h | 5 +++++ kernel/bpf/btf.c | 18 +++++++++--------- kernel/bpf/verifier.c | 5 ----- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 5742e9c0a7b8..d3ea9ef04767 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -738,6 +738,11 @@ struct bpf_verifier_env { char tmp_str_buf[TMP_STR_BUF_LEN]; }; +static inline struct bpf_func_info_aux *subprog_aux(struct bpf_verifier_env *env, int subprog) +{ + return &env->prog->aux->func_info_aux[subprog]; +} + static inline struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env, int subprog) { return &env->subprog_info[subprog]; diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 33d9a1c73f6e..d321340e16f1 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6914,6 +6914,7 @@ int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, */ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) { + bool is_global = subprog_aux(env, subprog)->linkage == BTF_FUNC_GLOBAL; struct bpf_subprog_info *sub = subprog_info(env, subprog); struct bpf_verifier_log *log = &env->log; struct bpf_prog *prog = env->prog; @@ -6927,14 +6928,15 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) if (sub->args_cached) return 0; - if (!prog->aux->func_info || - prog->aux->func_info_aux[subprog].linkage != BTF_FUNC_GLOBAL) { + if (!prog->aux->func_info) { bpf_log(log, "Verifier bug\n"); return -EFAULT; } btf_id = prog->aux->func_info[subprog].type_id; if (!btf_id) { + if (!is_global) /* not fatal for static funcs */ + return -EINVAL; bpf_log(log, "Global functions need valid BTF\n"); return -EFAULT; } @@ -6990,16 +6992,14 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) sub->args[i].arg_type = ARG_ANYTHING; continue; } - if (btf_type_is_ptr(t)) { + if (btf_type_is_ptr(t) && btf_get_prog_ctx_type(log, btf, t, prog_type, i)) { + sub->args[i].arg_type = ARG_PTR_TO_CTX; + continue; + } + if (is_global && btf_type_is_ptr(t)) { u32 mem_size; - if (btf_get_prog_ctx_type(log, btf, t, prog_type, i)) { - sub->args[i].arg_type = ARG_PTR_TO_CTX; - continue; - } - t = btf_type_skip_modifiers(btf, t->type, NULL); - ref_t = btf_resolve_size(btf, t, &mem_size); if (IS_ERR(ref_t)) { bpf_log(log, diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 540e4336290a..667cbf39087f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -437,11 +437,6 @@ static const char *subprog_name(const struct bpf_verifier_env *env, int subprog) return btf_type_name(env->prog->aux->btf, info->type_id); } -static struct bpf_func_info_aux *subprog_aux(const struct bpf_verifier_env *env, int subprog) -{ - return &env->prog->aux->func_info_aux[subprog]; -} - static void mark_subprog_exc_cb(struct bpf_verifier_env *env, int subprog) { struct bpf_subprog_info *info = subprog_info(env, subprog); From patchwork Fri Dec 15 01:13:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493880 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0b-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9608F363 for ; Fri, 15 Dec 2023 01:16:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0109332.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3BEJRHVc020617 for ; Thu, 14 Dec 2023 17:16:39 -0800 Received: from mail.thefacebook.com ([163.114.132.120]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3uynmqspgn-4 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:16:38 -0800 Received: from twshared51573.38.frc1.facebook.com (2620:10d:c085:208::11) by mail.thefacebook.com (2620:10d:c085:11d::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:16:35 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id 8A2323D2CCEE8; Thu, 14 Dec 2023 17:13:43 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 04/10] bpf: move subprog call logic back to verifier.c Date: Thu, 14 Dec 2023 17:13:28 -0800 Message-ID: <20231215011334.2307144-5-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: JvL9MfozMfbpW7ChgMGFA0ff4fylEvKh X-Proofpoint-GUID: JvL9MfozMfbpW7ChgMGFA0ff4fylEvKh X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Subprog call logic in btf_check_subprog_call() currently has both a lot of BTF parsing logic (which is, presumably, what justified putting it into btf.c), but also a bunch of register state checks, some of each utilize deep verifier logic helpers, necessarily exported from verifier.c: check_ptr_off_reg(), check_func_arg_reg_off(), and check_mem_reg(). Going forward, btf_check_subprog_call() will have a minimum of BTF-related logic, but will get more internal verifier logic related to register state manipulation. So move it into verifier.c to minimize amount of verifier-specific logic exposed to btf.c. We do this move before refactoring btf_check_func_arg_match() to preserve as much history post-refactoring as possible. No functional changes. Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- include/linux/bpf.h | 2 - include/linux/bpf_verifier.h | 8 -- kernel/bpf/btf.c | 139 ------------------------------- kernel/bpf/verifier.c | 153 +++++++++++++++++++++++++++++++++-- 4 files changed, 146 insertions(+), 156 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index cf80cfe9f0be..1dce146a5615 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2491,8 +2491,6 @@ int btf_distill_func_proto(struct bpf_verifier_log *log, struct btf_func_model *m); struct bpf_reg_state; -int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, - struct bpf_reg_state *regs); int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog); int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *prog, struct btf *btf, const struct btf_type *t); diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index d3ea9ef04767..d07d857ca67f 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -785,14 +785,6 @@ bpf_prog_offload_replace_insn(struct bpf_verifier_env *env, u32 off, void bpf_prog_offload_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt); -int check_ptr_off_reg(struct bpf_verifier_env *env, - const struct bpf_reg_state *reg, int regno); -int check_func_arg_reg_off(struct bpf_verifier_env *env, - const struct bpf_reg_state *reg, int regno, - enum bpf_arg_type arg_type); -int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, - u32 regno, u32 mem_size); - /* this lives here instead of in bpf.h because it needs to dereference tgt_prog */ static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog, struct btf *btf, u32 btf_id) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index d321340e16f1..341811bcca53 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6765,145 +6765,6 @@ int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *pr return btf_check_func_type_match(log, btf1, t1, btf2, t2); } -static int btf_check_func_arg_match(struct bpf_verifier_env *env, - const struct btf *btf, u32 func_id, - struct bpf_reg_state *regs, - bool ptr_to_mem_ok) -{ - enum bpf_prog_type prog_type = resolve_prog_type(env->prog); - struct bpf_verifier_log *log = &env->log; - const char *func_name, *ref_tname; - const struct btf_type *t, *ref_t; - const struct btf_param *args; - u32 i, nargs, ref_id; - int ret; - - t = btf_type_by_id(btf, func_id); - if (!t || !btf_type_is_func(t)) { - /* These checks were already done by the verifier while loading - * struct bpf_func_info or in add_kfunc_call(). - */ - bpf_log(log, "BTF of func_id %u doesn't point to KIND_FUNC\n", - func_id); - return -EFAULT; - } - func_name = btf_name_by_offset(btf, t->name_off); - - t = btf_type_by_id(btf, t->type); - if (!t || !btf_type_is_func_proto(t)) { - bpf_log(log, "Invalid BTF of func %s\n", func_name); - return -EFAULT; - } - args = (const struct btf_param *)(t + 1); - nargs = btf_type_vlen(t); - if (nargs > MAX_BPF_FUNC_REG_ARGS) { - bpf_log(log, "Function %s has %d > %d args\n", func_name, nargs, - MAX_BPF_FUNC_REG_ARGS); - return -EINVAL; - } - - /* check that BTF function arguments match actual types that the - * verifier sees. - */ - for (i = 0; i < nargs; i++) { - enum bpf_arg_type arg_type = ARG_DONTCARE; - u32 regno = i + 1; - struct bpf_reg_state *reg = ®s[regno]; - - t = btf_type_skip_modifiers(btf, args[i].type, NULL); - if (btf_type_is_scalar(t)) { - if (reg->type == SCALAR_VALUE) - continue; - bpf_log(log, "R%d is not a scalar\n", regno); - return -EINVAL; - } - - if (!btf_type_is_ptr(t)) { - bpf_log(log, "Unrecognized arg#%d type %s\n", - i, btf_type_str(t)); - return -EINVAL; - } - - ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); - ref_tname = btf_name_by_offset(btf, ref_t->name_off); - - ret = check_func_arg_reg_off(env, reg, regno, arg_type); - if (ret < 0) - return ret; - - if (btf_get_prog_ctx_type(log, btf, t, prog_type, i)) { - /* If function expects ctx type in BTF check that caller - * is passing PTR_TO_CTX. - */ - if (reg->type != PTR_TO_CTX) { - bpf_log(log, - "arg#%d expected pointer to ctx, but got %s\n", - i, btf_type_str(t)); - return -EINVAL; - } - } else if (ptr_to_mem_ok) { - const struct btf_type *resolve_ret; - u32 type_size; - - resolve_ret = btf_resolve_size(btf, ref_t, &type_size); - if (IS_ERR(resolve_ret)) { - bpf_log(log, - "arg#%d reference type('%s %s') size cannot be determined: %ld\n", - i, btf_type_str(ref_t), ref_tname, - PTR_ERR(resolve_ret)); - return -EINVAL; - } - - if (check_mem_reg(env, reg, regno, type_size)) - return -EINVAL; - } else { - bpf_log(log, "reg type unsupported for arg#%d function %s#%d\n", i, - func_name, func_id); - return -EINVAL; - } - } - - return 0; -} - -/* Compare BTF of a function call with given bpf_reg_state. - * Returns: - * EFAULT - there is a verifier bug. Abort verification. - * EINVAL - there is a type mismatch or BTF is not available. - * 0 - BTF matches with what bpf_reg_state expects. - * Only PTR_TO_CTX and SCALAR_VALUE states are recognized. - */ -int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, - struct bpf_reg_state *regs) -{ - struct bpf_prog *prog = env->prog; - struct btf *btf = prog->aux->btf; - bool is_global; - u32 btf_id; - int err; - - if (!prog->aux->func_info) - return -EINVAL; - - btf_id = prog->aux->func_info[subprog].type_id; - if (!btf_id) - return -EFAULT; - - if (prog->aux->func_info_aux[subprog].unreliable) - return -EINVAL; - - is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL; - err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global); - - /* Compiler optimizations can remove arguments from static functions - * or mismatched type can be passed into a global function. - * In such cases mark the function as unreliable from BTF point of view. - */ - if (err) - prog->aux->func_info_aux[subprog].unreliable = true; - return err; -} - /* Process BTF of a function to produce high-level expectation of function * arguments (like ARG_PTR_TO_CTX, or ARG_PTR_TO_MEM, etc). This information * is cached in subprog info for reuse. diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 667cbf39087f..b5237ddb00bf 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5134,8 +5134,8 @@ static int __check_ptr_off_reg(struct bpf_verifier_env *env, return 0; } -int check_ptr_off_reg(struct bpf_verifier_env *env, - const struct bpf_reg_state *reg, int regno) +static int check_ptr_off_reg(struct bpf_verifier_env *env, + const struct bpf_reg_state *reg, int regno) { return __check_ptr_off_reg(env, reg, regno, false); } @@ -7307,8 +7307,8 @@ static int check_mem_size_reg(struct bpf_verifier_env *env, return err; } -int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, - u32 regno, u32 mem_size) +static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + u32 regno, u32 mem_size) { bool may_be_null = type_may_be_null(reg->type); struct bpf_reg_state saved_reg; @@ -8293,9 +8293,9 @@ reg_find_field_offset(const struct bpf_reg_state *reg, s32 off, u32 fields) return field; } -int check_func_arg_reg_off(struct bpf_verifier_env *env, - const struct bpf_reg_state *reg, int regno, - enum bpf_arg_type arg_type) +static int check_func_arg_reg_off(struct bpf_verifier_env *env, + const struct bpf_reg_state *reg, int regno, + enum bpf_arg_type arg_type) { u32 type = reg->type; @@ -9256,6 +9256,145 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls return err; } +static int btf_check_func_arg_match(struct bpf_verifier_env *env, + const struct btf *btf, u32 func_id, + struct bpf_reg_state *regs, + bool ptr_to_mem_ok) +{ + enum bpf_prog_type prog_type = resolve_prog_type(env->prog); + struct bpf_verifier_log *log = &env->log; + const char *func_name, *ref_tname; + const struct btf_type *t, *ref_t; + const struct btf_param *args; + u32 i, nargs, ref_id; + int ret; + + t = btf_type_by_id(btf, func_id); + if (!t || !btf_type_is_func(t)) { + /* These checks were already done by the verifier while loading + * struct bpf_func_info or in add_kfunc_call(). + */ + bpf_log(log, "BTF of func_id %u doesn't point to KIND_FUNC\n", + func_id); + return -EFAULT; + } + func_name = btf_name_by_offset(btf, t->name_off); + + t = btf_type_by_id(btf, t->type); + if (!t || !btf_type_is_func_proto(t)) { + bpf_log(log, "Invalid BTF of func %s\n", func_name); + return -EFAULT; + } + args = (const struct btf_param *)(t + 1); + nargs = btf_type_vlen(t); + if (nargs > MAX_BPF_FUNC_REG_ARGS) { + bpf_log(log, "Function %s has %d > %d args\n", func_name, nargs, + MAX_BPF_FUNC_REG_ARGS); + return -EINVAL; + } + + /* check that BTF function arguments match actual types that the + * verifier sees. + */ + for (i = 0; i < nargs; i++) { + enum bpf_arg_type arg_type = ARG_DONTCARE; + u32 regno = i + 1; + struct bpf_reg_state *reg = ®s[regno]; + + t = btf_type_skip_modifiers(btf, args[i].type, NULL); + if (btf_type_is_scalar(t)) { + if (reg->type == SCALAR_VALUE) + continue; + bpf_log(log, "R%d is not a scalar\n", regno); + return -EINVAL; + } + + if (!btf_type_is_ptr(t)) { + bpf_log(log, "Unrecognized arg#%d type %s\n", + i, btf_type_str(t)); + return -EINVAL; + } + + ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); + ref_tname = btf_name_by_offset(btf, ref_t->name_off); + + ret = check_func_arg_reg_off(env, reg, regno, arg_type); + if (ret < 0) + return ret; + + if (btf_get_prog_ctx_type(log, btf, t, prog_type, i)) { + /* If function expects ctx type in BTF check that caller + * is passing PTR_TO_CTX. + */ + if (reg->type != PTR_TO_CTX) { + bpf_log(log, + "arg#%d expected pointer to ctx, but got %s\n", + i, btf_type_str(t)); + return -EINVAL; + } + } else if (ptr_to_mem_ok) { + const struct btf_type *resolve_ret; + u32 type_size; + + resolve_ret = btf_resolve_size(btf, ref_t, &type_size); + if (IS_ERR(resolve_ret)) { + bpf_log(log, + "arg#%d reference type('%s %s') size cannot be determined: %ld\n", + i, btf_type_str(ref_t), ref_tname, + PTR_ERR(resolve_ret)); + return -EINVAL; + } + + if (check_mem_reg(env, reg, regno, type_size)) + return -EINVAL; + } else { + bpf_log(log, "reg type unsupported for arg#%d function %s#%d\n", i, + func_name, func_id); + return -EINVAL; + } + } + + return 0; +} + +/* Compare BTF of a function call with given bpf_reg_state. + * Returns: + * EFAULT - there is a verifier bug. Abort verification. + * EINVAL - there is a type mismatch or BTF is not available. + * 0 - BTF matches with what bpf_reg_state expects. + * Only PTR_TO_CTX and SCALAR_VALUE states are recognized. + */ +static int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, + struct bpf_reg_state *regs) +{ + struct bpf_prog *prog = env->prog; + struct btf *btf = prog->aux->btf; + bool is_global; + u32 btf_id; + int err; + + if (!prog->aux->func_info) + return -EINVAL; + + btf_id = prog->aux->func_info[subprog].type_id; + if (!btf_id) + return -EFAULT; + + if (prog->aux->func_info_aux[subprog].unreliable) + return -EINVAL; + + is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL; + err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global); + + /* Compiler optimizations can remove arguments from static functions + * or mismatched type can be passed into a global function. + * In such cases mark the function as unreliable from BTF point of view. + */ + if (err) + prog->aux->func_info_aux[subprog].unreliable = true; + return err; +} + static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *insn, int insn_idx, int subprog, set_callee_state_fn set_callee_state_cb) From patchwork Fri Dec 15 01:13:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493876 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B193A624 for ; Fri, 15 Dec 2023 01:14:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0044012.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3BF0eIa2026727 for ; Thu, 14 Dec 2023 17:14:02 -0800 Received: from maileast.thefacebook.com ([163.114.130.16]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3v0cf8r6h7-12 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:14:01 -0800 Received: from twshared19681.14.frc2.facebook.com (2620:10d:c0a8:1c::1b) by mail.thefacebook.com (2620:10d:c0a8:83::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:13:56 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id 967DC3D2CCF03; Thu, 14 Dec 2023 17:13:45 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 05/10] bpf: reuse subprog argument parsing logic for subprog call checks Date: Thu, 14 Dec 2023 17:13:29 -0800 Message-ID: <20231215011334.2307144-6-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: 4zfa1f1V8oivK5-qF7i9oMrV4Q39xZIP X-Proofpoint-GUID: 4zfa1f1V8oivK5-qF7i9oMrV4Q39xZIP X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Remove duplicated BTF parsing logic when it comes to subprog call check. Instead, use (potentially cached) results of btf_prepare_func_args() to abstract away expectations of each subprog argument in generic terms (e.g., "this is pointer to context", or "this is a pointer to memory of size X"), and then use those simple high-level argument type expectations to validate actual register states to check if they match expectations. Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- kernel/bpf/verifier.c | 110 +++++------------- .../selftests/bpf/progs/test_global_func5.c | 2 +- 2 files changed, 31 insertions(+), 81 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index b5237ddb00bf..612e76a1034d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9256,101 +9256,54 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls return err; } -static int btf_check_func_arg_match(struct bpf_verifier_env *env, - const struct btf *btf, u32 func_id, - struct bpf_reg_state *regs, - bool ptr_to_mem_ok) +static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, + const struct btf *btf, + struct bpf_reg_state *regs) { - enum bpf_prog_type prog_type = resolve_prog_type(env->prog); + struct bpf_subprog_info *sub = subprog_info(env, subprog); struct bpf_verifier_log *log = &env->log; - const char *func_name, *ref_tname; - const struct btf_type *t, *ref_t; - const struct btf_param *args; - u32 i, nargs, ref_id; + u32 i; int ret; - t = btf_type_by_id(btf, func_id); - if (!t || !btf_type_is_func(t)) { - /* These checks were already done by the verifier while loading - * struct bpf_func_info or in add_kfunc_call(). - */ - bpf_log(log, "BTF of func_id %u doesn't point to KIND_FUNC\n", - func_id); - return -EFAULT; - } - func_name = btf_name_by_offset(btf, t->name_off); - - t = btf_type_by_id(btf, t->type); - if (!t || !btf_type_is_func_proto(t)) { - bpf_log(log, "Invalid BTF of func %s\n", func_name); - return -EFAULT; - } - args = (const struct btf_param *)(t + 1); - nargs = btf_type_vlen(t); - if (nargs > MAX_BPF_FUNC_REG_ARGS) { - bpf_log(log, "Function %s has %d > %d args\n", func_name, nargs, - MAX_BPF_FUNC_REG_ARGS); - return -EINVAL; - } + ret = btf_prepare_func_args(env, subprog); + if (ret) + return ret; /* check that BTF function arguments match actual types that the * verifier sees. */ - for (i = 0; i < nargs; i++) { - enum bpf_arg_type arg_type = ARG_DONTCARE; + for (i = 0; i < sub->arg_cnt; i++) { u32 regno = i + 1; struct bpf_reg_state *reg = ®s[regno]; + struct bpf_subprog_arg_info *arg = &sub->args[i]; - t = btf_type_skip_modifiers(btf, args[i].type, NULL); - if (btf_type_is_scalar(t)) { - if (reg->type == SCALAR_VALUE) - continue; - bpf_log(log, "R%d is not a scalar\n", regno); - return -EINVAL; - } - - if (!btf_type_is_ptr(t)) { - bpf_log(log, "Unrecognized arg#%d type %s\n", - i, btf_type_str(t)); - return -EINVAL; - } - - ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); - ref_tname = btf_name_by_offset(btf, ref_t->name_off); - - ret = check_func_arg_reg_off(env, reg, regno, arg_type); - if (ret < 0) - return ret; - - if (btf_get_prog_ctx_type(log, btf, t, prog_type, i)) { + if (arg->arg_type == ARG_ANYTHING) { + if (reg->type != SCALAR_VALUE) { + bpf_log(log, "R%d is not a scalar\n", regno); + return -EINVAL; + } + } else if (arg->arg_type == ARG_PTR_TO_CTX) { + ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE); + if (ret < 0) + return ret; /* If function expects ctx type in BTF check that caller * is passing PTR_TO_CTX. */ if (reg->type != PTR_TO_CTX) { - bpf_log(log, - "arg#%d expected pointer to ctx, but got %s\n", - i, btf_type_str(t)); - return -EINVAL; - } - } else if (ptr_to_mem_ok) { - const struct btf_type *resolve_ret; - u32 type_size; - - resolve_ret = btf_resolve_size(btf, ref_t, &type_size); - if (IS_ERR(resolve_ret)) { - bpf_log(log, - "arg#%d reference type('%s %s') size cannot be determined: %ld\n", - i, btf_type_str(ref_t), ref_tname, - PTR_ERR(resolve_ret)); + bpf_log(log, "arg#%d expects pointer to ctx\n", i); return -EINVAL; } + } else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) { + ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE); + if (ret < 0) + return ret; - if (check_mem_reg(env, reg, regno, type_size)) + if (check_mem_reg(env, reg, regno, arg->mem_size)) return -EINVAL; } else { - bpf_log(log, "reg type unsupported for arg#%d function %s#%d\n", i, - func_name, func_id); - return -EINVAL; + bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n", + i, arg->arg_type); + return -EFAULT; } } @@ -9365,11 +9318,10 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, * Only PTR_TO_CTX and SCALAR_VALUE states are recognized. */ static int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, - struct bpf_reg_state *regs) + struct bpf_reg_state *regs) { struct bpf_prog *prog = env->prog; struct btf *btf = prog->aux->btf; - bool is_global; u32 btf_id; int err; @@ -9383,9 +9335,7 @@ static int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, if (prog->aux->func_info_aux[subprog].unreliable) return -EINVAL; - is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL; - err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global); - + err = btf_check_func_arg_match(env, subprog, btf, regs); /* Compiler optimizations can remove arguments from static functions * or mismatched type can be passed into a global function. * In such cases mark the function as unreliable from BTF point of view. diff --git a/tools/testing/selftests/bpf/progs/test_global_func5.c b/tools/testing/selftests/bpf/progs/test_global_func5.c index cc55aedaf82d..257c0569ff98 100644 --- a/tools/testing/selftests/bpf/progs/test_global_func5.c +++ b/tools/testing/selftests/bpf/progs/test_global_func5.c @@ -26,7 +26,7 @@ int f3(int val, struct __sk_buff *skb) } SEC("tc") -__failure __msg("expected pointer to ctx, but got PTR") +__failure __msg("expects pointer to ctx") int global_func5(struct __sk_buff *skb) { return f1(skb) + f2(2, skb) + f3(3, skb); From patchwork Fri Dec 15 01:13:30 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493873 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 280147E4 for ; Fri, 15 Dec 2023 01:13:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0044012.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3BF0eREL027647 for ; Thu, 14 Dec 2023 17:13:54 -0800 Received: from maileast.thefacebook.com ([163.114.130.16]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3v0cf8r6h5-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:13:54 -0800 Received: from twshared10507.42.prn1.facebook.com (2620:10d:c0a8:1c::11) by mail.thefacebook.com (2620:10d:c0a8:82::b) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:13:51 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id A4DFD3D2CCF2E; Thu, 14 Dec 2023 17:13:47 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 06/10] bpf: support 'arg:xxx' btf_decl_tag-based hints for global subprog args Date: Thu, 14 Dec 2023 17:13:30 -0800 Message-ID: <20231215011334.2307144-7-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: Vvv0eX0-nSzSBQRVCwQQXZTh3m8hS0UY X-Proofpoint-GUID: Vvv0eX0-nSzSBQRVCwQQXZTh3m8hS0UY X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Add support for annotating global BPF subprog arguments to provide more information about expected semantics of the argument. Currently, verifier relies purely on argument's BTF type information, and supports three general use cases: scalar, pointer-to-context, and pointer-to-fixed-size-memory. Scalar and pointer-to-fixed-mem work well in practice and are quite natural to use. But pointer-to-context is a bit problematic, as typical BPF users don't realize that they need to use a special type name to signal to verifier that argument is not just some pointer, but actually a PTR_TO_CTX. Further, even if users do know which type to use, it is limiting in situations where the same BPF program logic is used across few different program types. Common case is kprobes, tracepoints, and perf_event programs having a helper to send some data over BPF perf buffer. bpf_perf_event_output() requires `ctx` argument, and so it's quite cumbersome to share such global subprog across few BPF programs of different types, necessitating extra static subprog that is context type-agnostic. Long story short, there is a need to go beyond types and allow users to add hints to global subprog arguments to define expectations. This patch adds such support for two initial special tags: - pointer to context; - non-null qualifier for generic pointer arguments. All of the above came up in practice already and seem generally useful additions. Non-null qualifier is an often requested feature, which currently has to be worked around by having unnecessary NULL checks inside subprogs even if we know that arguments are never NULL. Pointer to context was discussed earlier. As for implementation, we utilize btf_decl_tag attribute and set up an "arg:xxx" convention to specify argument hint. As such: - btf_decl_tag("arg:ctx") is a PTR_TO_CTX hint; - btf_decl_tag("arg:nonnull") marks pointer argument as not allowed to be NULL, making NULL check inside global subprog unnecessary. Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- kernel/bpf/btf.c | 44 +++++++++++++++++++++++++++++++++++++------ kernel/bpf/verifier.c | 5 ++++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 341811bcca53..c0fc8977be97 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6782,7 +6782,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) enum bpf_prog_type prog_type = prog->type; struct btf *btf = prog->aux->btf; const struct btf_param *args; - const struct btf_type *t, *ref_t; + const struct btf_type *t, *ref_t, *fn_t; u32 i, nargs, btf_id; const char *tname; @@ -6802,8 +6802,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) return -EFAULT; } - t = btf_type_by_id(btf, btf_id); - if (!t || !btf_type_is_func(t)) { + fn_t = btf_type_by_id(btf, btf_id); + if (!fn_t || !btf_type_is_func(fn_t)) { /* These checks were already done by the verifier while loading * struct bpf_func_info */ @@ -6811,7 +6811,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) subprog); return -EFAULT; } - tname = btf_name_by_offset(btf, t->name_off); + tname = btf_name_by_offset(btf, fn_t->name_off); if (prog->aux->func_info_aux[subprog].unreliable) { bpf_log(log, "Verifier bug in function %s()\n", tname); @@ -6820,7 +6820,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) if (prog_type == BPF_PROG_TYPE_EXT) prog_type = prog->aux->dst_prog->type; - t = btf_type_by_id(btf, t->type); + t = btf_type_by_id(btf, fn_t->type); if (!t || !btf_type_is_func_proto(t)) { bpf_log(log, "Invalid type of function %s()\n", tname); return -EFAULT; @@ -6846,7 +6846,35 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) * Only PTR_TO_CTX and SCALAR are supported atm. */ for (i = 0; i < nargs; i++) { + bool is_nonnull = false; + const char *tag; + t = btf_type_by_id(btf, args[i].type); + + tag = btf_find_decl_tag_value(btf, fn_t, i, "arg:"); + if (IS_ERR(tag) && PTR_ERR(tag) == -ENOENT) { + tag = NULL; + } else if (IS_ERR(tag)) { + bpf_log(log, "arg#%d type's tag fetching failure: %ld\n", i, PTR_ERR(tag)); + return PTR_ERR(tag); + } + /* 'arg:' decl_tag takes precedence over derivation of + * register type from BTF type itself + */ + if (tag) { + /* disallow arg tags in static subprogs */ + if (!is_global) { + bpf_log(log, "arg#%d type tag is not supported in static functions\n", i); + return -EOPNOTSUPP; + } + if (strcmp(tag, "ctx") == 0) { + sub->args[i].arg_type = ARG_PTR_TO_CTX; + continue; + } + if (strcmp(tag, "nonnull") == 0) + is_nonnull = true; + } + while (btf_type_is_modifier(t)) t = btf_type_by_id(btf, t->type); if (btf_type_is_int(t) || btf_is_any_enum(t)) { @@ -6870,10 +6898,14 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) return -EINVAL; } - sub->args[i].arg_type = ARG_PTR_TO_MEM_OR_NULL; + sub->args[i].arg_type = is_nonnull ? ARG_PTR_TO_MEM : ARG_PTR_TO_MEM_OR_NULL; sub->args[i].mem_size = mem_size; continue; } + if (is_nonnull) { + bpf_log(log, "arg#%d marked as non-null, but is not a pointer type\n", i); + return -EINVAL; + } bpf_log(log, "Arg#%d type %s in %s() is not supported yet.\n", i, btf_type_str(t), tname); return -EINVAL; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 612e76a1034d..e64210d8c617 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9297,9 +9297,12 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE); if (ret < 0) return ret; - if (check_mem_reg(env, reg, regno, arg->mem_size)) return -EINVAL; + if (!(arg->arg_type & PTR_MAYBE_NULL) && (reg->type & PTR_MAYBE_NULL)) { + bpf_log(log, "arg#%d is expected to be non-NULL\n", i); + return -EINVAL; + } } else { bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n", i, arg->arg_type); From patchwork Fri Dec 15 01:13:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493877 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0a-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 255747E4 for ; Fri, 15 Dec 2023 01:14:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0001303.ppops.net [127.0.0.1]) by m0001303.ppops.net (8.17.1.19/8.17.1.19) with ESMTP id 3BEMrINl019712 for ; Thu, 14 Dec 2023 17:14:02 -0800 Received: from mail.thefacebook.com ([163.114.132.120]) by m0001303.ppops.net (PPS) with ESMTPS id 3v0avv8xbb-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:14:01 -0800 Received: from twshared17205.35.frc1.facebook.com (2620:10d:c085:208::11) by mail.thefacebook.com (2620:10d:c085:11d::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:14:00 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id B140C3D2CCF38; Thu, 14 Dec 2023 17:13:49 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 07/10] bpf: add support for passing dynptr pointer to global subprog Date: Thu, 14 Dec 2023 17:13:31 -0800 Message-ID: <20231215011334.2307144-8-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: cIC2kg3gF7YGuIjo9wJWEBHWXxkERffO X-Proofpoint-GUID: cIC2kg3gF7YGuIjo9wJWEBHWXxkERffO X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Add ability to pass a pointer to dynptr into global functions. This allows to have global subprogs that accept and work with generic dynptrs that are created by caller. Dynptr argument is detected based on the name of a struct type, if it's "bpf_dynptr", it's assumed to be a proper dynptr pointer. Both actual struct and forward struct declaration types are supported. This is conceptually exactly the same semantics as bpf_user_ringbuf_drain()'s use of dynptr to pass a variable-sized pointer to ringbuf record. So we heavily rely on CONST_PTR_TO_DYNPTR bits of already existing logic in the verifier. During global subprog validation, we mark such CONST_PTR_TO_DYNPTR as having LOCAL type, as that's the most unassuming type of dynptr and it doesn't have any special helpers that can try to free or acquire extra references (unlike skb, xdp, or ringbuf dynptr). So that seems like a safe "choice" to make from correctness standpoint. It's still possible to pass any type of dynptr to such subprog, though, because generic dynptr helpers, like getting data/slice pointers, read/write memory copying routines, dynptr adjustment and getter routines all work correctly with any type of dynptr. Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- kernel/bpf/btf.c | 23 +++++++++++++++++++++++ kernel/bpf/verifier.c | 7 +++++++ 2 files changed, 30 insertions(+) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index c0fc8977be97..51e8b4bee0c8 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6765,6 +6765,25 @@ int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *pr return btf_check_func_type_match(log, btf1, t1, btf2, t2); } +static bool btf_is_dynptr_ptr(const struct btf *btf, const struct btf_type *t) +{ + const char *name; + + t = btf_type_by_id(btf, t->type); /* skip PTR */ + + while (btf_type_is_modifier(t)) + t = btf_type_by_id(btf, t->type); + + /* allow either struct or struct forward declaration */ + if (btf_type_is_struct(t) || + (btf_type_is_fwd(t) && btf_type_kflag(t) == 0)) { + name = btf_str_by_offset(btf, t->name_off); + return name && strcmp(name, "bpf_dynptr") == 0; + } + + return false; +} + /* Process BTF of a function to produce high-level expectation of function * arguments (like ARG_PTR_TO_CTX, or ARG_PTR_TO_MEM, etc). This information * is cached in subprog info for reuse. @@ -6885,6 +6904,10 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) sub->args[i].arg_type = ARG_PTR_TO_CTX; continue; } + if (btf_type_is_ptr(t) && btf_is_dynptr_ptr(btf, t)) { + sub->args[i].arg_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY; + continue; + } if (is_global && btf_type_is_ptr(t)) { u32 mem_size; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e64210d8c617..e4838c9b79db 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9303,6 +9303,10 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, bpf_log(log, "arg#%d is expected to be non-NULL\n", i); return -EINVAL; } + } else if (arg->arg_type == (ARG_PTR_TO_DYNPTR | MEM_RDONLY)) { + ret = process_dynptr_func(env, regno, -1, arg->arg_type, 0); + if (ret) + return ret; } else { bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n", i, arg->arg_type); @@ -20023,6 +20027,9 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) } else if (arg->arg_type == ARG_ANYTHING) { reg->type = SCALAR_VALUE; mark_reg_unknown(env, regs, i); + } else if (arg->arg_type == (ARG_PTR_TO_DYNPTR | MEM_RDONLY)) { + /* assume unspecial LOCAL dynptr type */ + __mark_dynptr_reg(reg, BPF_DYNPTR_TYPE_LOCAL, true, ++env->id_gen); } else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) { reg->type = PTR_TO_MEM; if (arg->arg_type & PTR_MAYBE_NULL) From patchwork Fri Dec 15 01:13:32 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493878 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0b-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8F477363 for ; Fri, 15 Dec 2023 01:14:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0109332.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3BEJjXHH021186 for ; Thu, 14 Dec 2023 17:14:07 -0800 Received: from mail.thefacebook.com ([163.114.132.120]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3uynmqsp30-4 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:14:07 -0800 Received: from twshared4634.37.frc1.facebook.com (2620:10d:c085:208::11) by mail.thefacebook.com (2620:10d:c085:11d::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:14:04 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id BD9913D2CCF58; Thu, 14 Dec 2023 17:13:51 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 08/10] libbpf: add __arg_xxx macros for annotating global func args Date: Thu, 14 Dec 2023 17:13:32 -0800 Message-ID: <20231215011334.2307144-9-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: SQ3WEC2AdzzHeTonSVDuwPRJFcORaVU8 X-Proofpoint-GUID: SQ3WEC2AdzzHeTonSVDuwPRJFcORaVU8 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Add a set of __arg_xxx macros which can be used to augment BPF global subprogs/functions with extra information for use by BPF verifier. Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- tools/lib/bpf/bpf_helpers.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h index 77ceea575dc7..2324cc42b017 100644 --- a/tools/lib/bpf/bpf_helpers.h +++ b/tools/lib/bpf/bpf_helpers.h @@ -188,6 +188,9 @@ enum libbpf_tristate { !!sym; \ }) +#define __arg_ctx __attribute__((btf_decl_tag("arg:ctx"))) +#define __arg_nonnull __attribute((btf_decl_tag("arg:nonnull"))) + #ifndef ___bpf_concat #define ___bpf_concat(a, b) a ## b #endif From patchwork Fri Dec 15 01:13:33 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493875 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0a-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 73D5C36F for ; Fri, 15 Dec 2023 01:14:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0001303.ppops.net [127.0.0.1]) by m0001303.ppops.net (8.17.1.19/8.17.1.19) with ESMTP id 3BEMrIBr019722 for ; Thu, 14 Dec 2023 17:14:00 -0800 Received: from maileast.thefacebook.com ([163.114.130.16]) by m0001303.ppops.net (PPS) with ESMTPS id 3v0avv8xb0-5 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:14:00 -0800 Received: from twshared19681.14.frc2.facebook.com (2620:10d:c0a8:1b::30) by mail.thefacebook.com (2620:10d:c0a8:82::b) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:13:56 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id C5E123D2CCF9F; Thu, 14 Dec 2023 17:13:53 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 09/10] selftests/bpf: add global subprog annotation tests Date: Thu, 14 Dec 2023 17:13:33 -0800 Message-ID: <20231215011334.2307144-10-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-ORIG-GUID: FQTC_Bj1XC2-zz1in_1VimebuoZPR87B X-Proofpoint-GUID: FQTC_Bj1XC2-zz1in_1VimebuoZPR87B X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Add test cases to validate semantics of global subprog argument annotations: - non-null pointers; - context argument; - const dynptr passing; - packet pointers (data, metadata, end). Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- .../bpf/progs/verifier_global_subprogs.c | 99 ++++++++++++++++++- 1 file changed, 95 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c index bd696a431244..9eeb2d89cda8 100644 --- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c +++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c @@ -1,12 +1,11 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */ -#include -#include -#include -#include +#include #include #include "bpf_misc.h" +#include "xdp_metadata.h" +#include "bpf_kfuncs.h" int arr[1]; int unkn_idx; @@ -98,4 +97,96 @@ int unguarded_unsupp_global_called(void) return global_unsupp(&x); } +long stack[128]; + +__weak int subprog_nullable_ptr_bad(int *p) +{ + return (*p) * 2; /* bad, missing null check */ +} + +SEC("?raw_tp") +__failure __log_level(2) +__msg("invalid mem access 'mem_or_null'") +int arg_tag_nullable_ptr_fail(void *ctx) +{ + int x = 42; + + return subprog_nullable_ptr_bad(&x); +} + +__noinline __weak int subprog_nonnull_ptr_good(int *p1 __arg_nonnull, int *p2 __arg_nonnull) +{ + return (*p1) * (*p2); /* good, no need for NULL checks */ +} + +int x = 47; + +SEC("?raw_tp") +__success __log_level(2) +int arg_tag_nonnull_ptr_good(void *ctx) +{ + int y = 74; + + return subprog_nonnull_ptr_good(&x, &y); +} + +/* this global subprog can be now called from many types of entry progs, each + * with different context type + */ +__weak int subprog_ctx_tag(void *ctx __arg_ctx) +{ + return bpf_get_stack(ctx, stack, sizeof(stack), 0); +} + +SEC("?raw_tp") +__success __log_level(2) +int arg_tag_ctx_raw_tp(void *ctx) +{ + return subprog_ctx_tag(ctx); +} + +SEC("?tp") +__success __log_level(2) +int arg_tag_ctx_tp(void *ctx) +{ + return subprog_ctx_tag(ctx); +} + +SEC("?kprobe") +__success __log_level(2) +int arg_tag_ctx_kprobe(void *ctx) +{ + return subprog_ctx_tag(ctx); +} + +__weak int subprog_dynptr(struct bpf_dynptr *dptr) +{ + long *d, t, buf[1] = {}; + + d = bpf_dynptr_data(dptr, 0, sizeof(long)); + if (!d) + return 0; + + t = *d + 1; + + d = bpf_dynptr_slice(dptr, 0, &buf, sizeof(long)); + if (!d) + return t; + + t = *d + 2; + + return t; +} + +SEC("?xdp") +__success __log_level(2) +int arg_tag_dynptr(struct xdp_md *ctx) +{ + struct bpf_dynptr dptr; + + bpf_dynptr_from_xdp(ctx, 0, &dptr); + + return subprog_dynptr(&dptr); +} + char _license[] SEC("license") = "GPL"; From patchwork Fri Dec 15 01:13:34 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13493879 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mx0a-00082601.pphosted.com (mx0a-00082601.pphosted.com [67.231.145.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5DDFD7E4 for ; Fri, 15 Dec 2023 01:14:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Received: from pps.filterd (m0109334.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3BEMD3fp028286 for ; Thu, 14 Dec 2023 17:14:15 -0800 Received: from maileast.thefacebook.com ([163.114.130.16]) by mx0a-00082601.pphosted.com (PPS) with ESMTPS id 3uynm6gugu-5 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Thu, 14 Dec 2023 17:14:14 -0800 Received: from twshared68648.02.prn6.facebook.com (2620:10d:c0a8:1b::30) by mail.thefacebook.com (2620:10d:c0a8:83::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 14 Dec 2023 17:14:09 -0800 Received: by devbig019.vll3.facebook.com (Postfix, from userid 137359) id D247A3D2CCFCD; Thu, 14 Dec 2023 17:13:55 -0800 (PST) From: Andrii Nakryiko To: , , , CC: , , Eduard Zingerman Subject: [PATCH v3 bpf-next 10/10] selftests/bpf: add freplace of BTF-unreliable main prog test Date: Thu, 14 Dec 2023 17:13:34 -0800 Message-ID: <20231215011334.2307144-11-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231215011334.2307144-1-andrii@kernel.org> References: <20231215011334.2307144-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-FB-Internal: Safe X-Proofpoint-GUID: nJd6LU_ExIFqIoNh7aUUAeR0Qet3uwc7 X-Proofpoint-ORIG-GUID: nJd6LU_ExIFqIoNh7aUUAeR0Qet3uwc7 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-14_17,2023-12-14_01,2023-05-22_02 X-Patchwork-Delegate: bpf@iogearbox.net Add a test validating that freplace'ing another main (entry) BPF program fails if the target BPF program doesn't have valid/expected func proto BTF. We extend fexit_bpf2bpf test to allow to specify expected log message for negative test cases (where freplace program is expected to fail to load). Acked-by: Eduard Zingerman Signed-off-by: Andrii Nakryiko --- .../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 30 +++++++++++++++++-- .../selftests/bpf/prog_tests/verifier.c | 2 ++ .../bpf/progs/freplace_unreliable_prog.c | 20 +++++++++++++ .../bpf/progs/verifier_btf_unreliable_prog.c | 20 +++++++++++++ 4 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/bpf/progs/freplace_unreliable_prog.c create mode 100644 tools/testing/selftests/bpf/progs/verifier_btf_unreliable_prog.c diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c index 8ec73fdfcdab..f29fc789c14b 100644 --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c @@ -348,7 +348,8 @@ static void test_func_sockmap_update(void) } static void test_obj_load_failure_common(const char *obj_file, - const char *target_obj_file) + const char *target_obj_file, + const char *exp_msg) { /* * standalone test that asserts failure to load freplace prog @@ -356,6 +357,7 @@ static void test_obj_load_failure_common(const char *obj_file, */ struct bpf_object *obj = NULL, *pkt_obj; struct bpf_program *prog; + char log_buf[64 * 1024]; int err, pkt_fd; __u32 duration = 0; @@ -374,11 +376,21 @@ static void test_obj_load_failure_common(const char *obj_file, err = bpf_program__set_attach_target(prog, pkt_fd, NULL); ASSERT_OK(err, "set_attach_target"); + log_buf[0] = '\0'; + if (exp_msg) + bpf_program__set_log_buf(prog, log_buf, sizeof(log_buf)); + if (env.verbosity > VERBOSE_NONE) + bpf_program__set_log_level(prog, 2); + /* It should fail to load the program */ err = bpf_object__load(obj); + if (env.verbosity > VERBOSE_NONE && exp_msg) /* we overtook log */ + printf("VERIFIER LOG:\n================\n%s\n================\n", log_buf); if (CHECK(!err, "bpf_obj_load should fail", "err %d\n", err)) goto close_prog; + if (exp_msg) + ASSERT_HAS_SUBSTR(log_buf, exp_msg, "fail_msg"); close_prog: bpf_object__close(obj); bpf_object__close(pkt_obj); @@ -388,14 +400,24 @@ static void test_func_replace_return_code(void) { /* test invalid return code in the replaced program */ test_obj_load_failure_common("./freplace_connect_v4_prog.bpf.o", - "./connect4_prog.bpf.o"); + "./connect4_prog.bpf.o", NULL); } static void test_func_map_prog_compatibility(void) { /* test with spin lock map value in the replaced program */ test_obj_load_failure_common("./freplace_attach_probe.bpf.o", - "./test_attach_probe.bpf.o"); + "./test_attach_probe.bpf.o", NULL); +} + +static void test_func_replace_unreliable(void) +{ + /* freplace'ing unreliable main prog should fail with error + * "Cannot replace static functions" + */ + test_obj_load_failure_common("freplace_unreliable_prog.bpf.o", + "./verifier_btf_unreliable_prog.bpf.o", + "Cannot replace static functions"); } static void test_func_replace_global_func(void) @@ -563,6 +585,8 @@ void serial_test_fexit_bpf2bpf(void) test_func_replace_return_code(); if (test__start_subtest("func_map_prog_compatibility")) test_func_map_prog_compatibility(); + if (test__start_subtest("func_replace_unreliable")) + test_func_replace_unreliable(); if (test__start_subtest("func_replace_multi")) test_func_replace_multi(); if (test__start_subtest("fmod_ret_freplace")) diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c index ac49ec25211d..d62c5bf00e71 100644 --- a/tools/testing/selftests/bpf/prog_tests/verifier.c +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c @@ -14,6 +14,7 @@ #include "verifier_bpf_get_stack.skel.h" #include "verifier_bswap.skel.h" #include "verifier_btf_ctx_access.skel.h" +#include "verifier_btf_unreliable_prog.skel.h" #include "verifier_cfg.skel.h" #include "verifier_cgroup_inv_retcode.skel.h" #include "verifier_cgroup_skb.skel.h" @@ -125,6 +126,7 @@ void test_verifier_bounds_mix_sign_unsign(void) { RUN(verifier_bounds_mix_sign_u void test_verifier_bpf_get_stack(void) { RUN(verifier_bpf_get_stack); } void test_verifier_bswap(void) { RUN(verifier_bswap); } void test_verifier_btf_ctx_access(void) { RUN(verifier_btf_ctx_access); } +void test_verifier_btf_unreliable_prog(void) { RUN(verifier_btf_unreliable_prog); } void test_verifier_cfg(void) { RUN(verifier_cfg); } void test_verifier_cgroup_inv_retcode(void) { RUN(verifier_cgroup_inv_retcode); } void test_verifier_cgroup_skb(void) { RUN(verifier_cgroup_skb); } diff --git a/tools/testing/selftests/bpf/progs/freplace_unreliable_prog.c b/tools/testing/selftests/bpf/progs/freplace_unreliable_prog.c new file mode 100644 index 000000000000..624078abf3de --- /dev/null +++ b/tools/testing/selftests/bpf/progs/freplace_unreliable_prog.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2020 Facebook + +#include "vmlinux.h" +#include +#include + +SEC("freplace/btf_unreliable_kprobe") +/* context type is what BPF verifier expects for kprobe context, but target + * program has `stuct whatever *ctx` argument, so freplace operation will be + * rejected with the following message: + * + * arg0 replace_btf_unreliable_kprobe(struct pt_regs *) doesn't match btf_unreliable_kprobe(struct whatever *) + */ +int replace_btf_unreliable_kprobe(bpf_user_pt_regs_t *ctx) +{ + return 0; +} + +char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/progs/verifier_btf_unreliable_prog.c b/tools/testing/selftests/bpf/progs/verifier_btf_unreliable_prog.c new file mode 100644 index 000000000000..36e033a2e02c --- /dev/null +++ b/tools/testing/selftests/bpf/progs/verifier_btf_unreliable_prog.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2017 Facebook + +#include "vmlinux.h" +#include +#include +#include +#include "bpf_misc.h" + +struct whatever {}; + +SEC("kprobe") +__success __log_level(2) +/* context type is wrong, making it impossible to freplace this program */ +int btf_unreliable_kprobe(struct whatever *ctx) +{ + return 0; +} + +char _license[] SEC("license") = "GPL";