Commit 5eccd2db authored by Andrii Nakryiko's avatar Andrii Nakryiko Committed by Alexei Starovoitov
Browse files

bpf: reuse btf_prepare_func_args() check for main program BTF validation 



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: default avatarEduard Zingerman <eddyz87@gmail.com>
Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20231215011334.2307144-3-andrii@kernel.org


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 4ba1d0f2

include/linux/bpf.h

+0 −2
Original line number Diff line number Diff line
@@ -2466,8 +2466,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);

kernel/bpf/btf.c

+3 −47
Original line number Diff line number Diff line
@@ -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.

kernel/bpf/verifier.c

+12 −13
Original line number Diff line number Diff line
@@ -19904,6 +19904,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;
@@ -19930,9 +19931,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;








@@ -19979,21 +19980,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);

























tools/testing/selftests/bpf/prog_tests/log_fixup.c



+2
−2






























Original line number
Diff line number
Diff line







@@ -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"))
































tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c



+1
−1






























Original line number
Diff line number
Diff line







@@ -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;