Commit 9cc31b3a authored by Alexei Starovoitov's avatar Alexei Starovoitov Committed by Daniel Borkmann
Browse files

bpf: Fix race in btf_resolve_helper_id() 



btf_resolve_helper_id() caching logic is a bit racy, since under root the
verifier can verify several programs in parallel. Fix it with READ/WRITE_ONCE.
Fix the type as well, since error is also recorded.

Fixes: a7658e1a ("bpf: Check types of arguments passed into helpers")
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarSong Liu <songliubraving@fb.com>
Acked-by: default avatarAndrii Nakryiko <andriin@fb.com>
Link: https://lore.kernel.org/bpf/20191114185720.1641606-15-ast@kernel.org
parent 9fd4a39d

include/linux/bpf.h

+3 −2
Original line number Diff line number Diff line
@@ -248,7 +248,7 @@ struct bpf_func_proto { 
		};

		enum bpf_arg_type arg_type[5];

	};

	u32 *btf_id; /* BTF ids of arguments */

	int *btf_id; /* BTF ids of arguments */

};



/* bpf_context is intentionally undefined structure. Pointer to bpf_context is
@@ -881,7 +881,8 @@ int btf_struct_access(struct bpf_verifier_log *log, 
		      const struct btf_type *t, int off, int size,

		      enum bpf_access_type atype,

		      u32 *next_btf_id);

u32 btf_resolve_helper_id(struct bpf_verifier_log *log, void *, int);

int btf_resolve_helper_id(struct bpf_verifier_log *log,

			  const struct bpf_func_proto *fn, int);



int btf_distill_func_proto(struct bpf_verifier_log *log,

			   struct btf *btf,

kernel/bpf/btf.c

+25 −1
Original line number Diff line number Diff line
@@ -3721,7 +3721,8 @@ again: 
	return -EINVAL;

}



u32 btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn, int arg)

static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,

				   int arg)

{

	char fnname[KSYM_SYMBOL_LEN + 4] = "btf_";

	const struct btf_param *args;
@@ -3789,6 +3790,29 @@ u32 btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn, int arg) 
	return btf_id;

}



int btf_resolve_helper_id(struct bpf_verifier_log *log,

			  const struct bpf_func_proto *fn, int arg)

{

	int *btf_id = &fn->btf_id[arg];

	int ret;



	if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID)

		return -EINVAL;



	ret = READ_ONCE(*btf_id);

	if (ret)

		return ret;

	/* ok to race the search. The result is the same */

	ret = __btf_resolve_helper_id(log, fn->func, arg);

	if (!ret) {

		/* Function argument cannot be type 'void' */

		bpf_log(log, "BTF resolution bug\n");

		return -EFAULT;

	}

	WRITE_ONCE(*btf_id, ret);

	return ret;

}



static int __get_type_size(struct btf *btf, u32 btf_id,

			   const struct btf_type **bad_type)

{

kernel/bpf/verifier.c

+3 −5
Original line number Diff line number Diff line
@@ -4147,11 +4147,9 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn 
	meta.func_id = func_id;

	/* check args */

	for (i = 0; i < 5; i++) {

		if (fn->arg_type[i] == ARG_PTR_TO_BTF_ID) {

			if (!fn->btf_id[i])

				fn->btf_id[i] = btf_resolve_helper_id(&env->log, fn->func, i);

			meta.btf_id = fn->btf_id[i];

		}

		err = btf_resolve_helper_id(&env->log, fn, i);

		if (err > 0)

			meta.btf_id = err;

		err = check_func_arg(env, BPF_REG_1 + i, fn->arg_type[i], &meta);

		if (err)

			return err;

net/core/filter.c

+1 −1
Original line number Diff line number Diff line
@@ -3816,7 +3816,7 @@ static const struct bpf_func_proto bpf_skb_event_output_proto = { 
	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,

};



static u32 bpf_skb_output_btf_ids[5];

static int bpf_skb_output_btf_ids[5];

const struct bpf_func_proto bpf_skb_output_proto = {

	.func		= bpf_skb_event_output,

	.gpl_only	= true,

CRA Git | Maintained and supported by SUSTech CRA and CCSE