Commit 4153b89b authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'refactor-check_func_arg' 



Lorenz Bauer says:

====================
Changes in v4:
- Output the desired type on BTF ID mismatch (Martin)

Changes in v3:
- Fix BTF_ID_LIST_SINGLE if BTF is disabled (Martin)
- Drop incorrect arg_btf_id in bpf_sk_storage.c (Martin)
- Check for arg_btf_id in check_func_proto (Martin)
- Drop incorrect PTR_TO_BTF_ID from fullsock_types (Martin)
- Introduce btf_seq_file_ids in bpf_trace.c to reduce duplication

Changes in v2:
- Make the series stand alone (Martin)
- Drop incorrect BTF_SET_START fix (Andrii)
- Only support a single BTF ID per argument (Martin)
- Introduce BTF_ID_LIST_SINGLE macro (Andrii)
- Skip check_ctx_reg iff register is NULL
- Change output of check_reg_type slightly, to avoid touching tests

Original cover letter:

Currently, check_func_arg has this pretty gnarly if statement that
compares the valid arg_type with the actualy reg_type. Sprinkled
in-between are checks for register_is_null, to short circuit these
tests if we're dealing with a nullable arg_type. There is also some
code for later bounds / access checking hidden away in there.

This series of patches refactors the function into something like this:

   if (reg_is_null && arg_type_is_nullable)
     skip type checking

   do type checking, including BTF validation

   do bounds / access checking

The type checking is now table driven, which makes it easy to extend
the acceptable types. Maybe more importantly, using a table makes it
easy to provide more helpful verifier output (see the last patch).
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 31f23a6a f79e7ea5

include/linux/bpf.h

+12 −9
Original line number Diff line number Diff line
@@ -292,6 +292,7 @@ enum bpf_arg_type { 
	ARG_PTR_TO_ALLOC_MEM,	/* pointer to dynamically allocated memory */

	ARG_PTR_TO_ALLOC_MEM_OR_NULL,	/* pointer to dynamically allocated memory or NULL */

	ARG_CONST_ALLOC_SIZE_OR_ZERO,	/* number of allocated bytes requested */

	__BPF_ARG_TYPE_MAX,

};



/* type of values returned from helper functions */
@@ -326,12 +327,16 @@ struct bpf_func_proto { 
		};

		enum bpf_arg_type arg_type[5];

	};

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

	bool (*check_btf_id)(u32 btf_id, u32 arg); /* if the argument btf_id is

						    * valid. Often used if more

						    * than one btf id is permitted

						    * for this argument.

						    */

	union {

		struct {

			u32 *arg1_btf_id;

			u32 *arg2_btf_id;

			u32 *arg3_btf_id;

			u32 *arg4_btf_id;

			u32 *arg5_btf_id;

		};

		u32 *arg_btf_id[5];

	};

	int *ret_btf_id; /* return value btf_id */

	bool (*allowed)(const struct bpf_prog *prog);

};
@@ -1385,8 +1390,6 @@ int btf_struct_access(struct bpf_verifier_log *log, 
		      u32 *next_btf_id);

bool btf_struct_ids_match(struct bpf_verifier_log *log,

			  int off, u32 id, u32 need_type_id);

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,
@@ -1905,6 +1908,6 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t, 
		       void *addr1, void *addr2);



struct btf_id_set;

bool btf_id_set_contains(struct btf_id_set *set, u32 id);

bool btf_id_set_contains(const struct btf_id_set *set, u32 id);



#endif /* _LINUX_BPF_H */

include/linux/btf_ids.h

+8 −0
Original line number Diff line number Diff line
@@ -76,6 +76,13 @@ extern u32 name[]; 
#define BTF_ID_LIST_GLOBAL(name)			\

__BTF_ID_LIST(name, globl)



/* The BTF_ID_LIST_SINGLE macro defines a BTF_ID_LIST with

 * a single entry.

 */

#define BTF_ID_LIST_SINGLE(name, prefix, typename)	\

	BTF_ID_LIST(name) \

	BTF_ID(prefix, typename)



/*

 * The BTF_ID_UNUSED macro defines 4 zero bytes.

 * It's used when we want to define 'unused' entry
@@ -140,6 +147,7 @@ extern struct btf_id_set name; 
#define BTF_ID(prefix, name)

#define BTF_ID_UNUSED

#define BTF_ID_LIST_GLOBAL(name) u32 name[1];

#define BTF_ID_LIST_SINGLE(name, prefix, typename) static u32 name[1];

#define BTF_SET_START(name) static struct btf_id_set name = { 0 };

#define BTF_SET_START_GLOBAL(name) static struct btf_id_set name = { 0 };

#define BTF_SET_END(name)

kernel/bpf/bpf_inode_storage.c

+3 −5
Original line number Diff line number Diff line
@@ -249,9 +249,7 @@ const struct bpf_map_ops inode_storage_map_ops = { 
	.map_owner_storage_ptr = inode_storage_ptr,

};



BTF_ID_LIST(bpf_inode_storage_btf_ids)

BTF_ID_UNUSED

BTF_ID(struct, inode)

BTF_ID_LIST_SINGLE(bpf_inode_storage_btf_ids, struct, inode)



const struct bpf_func_proto bpf_inode_storage_get_proto = {

	.func		= bpf_inode_storage_get,
@@ -259,9 +257,9 @@ const struct bpf_func_proto bpf_inode_storage_get_proto = { 
	.ret_type	= RET_PTR_TO_MAP_VALUE_OR_NULL,

	.arg1_type	= ARG_CONST_MAP_PTR,

	.arg2_type	= ARG_PTR_TO_BTF_ID,

	.arg2_btf_id	= &bpf_inode_storage_btf_ids[0],

	.arg3_type	= ARG_PTR_TO_MAP_VALUE_OR_NULL,

	.arg4_type	= ARG_ANYTHING,

	.btf_id		= bpf_inode_storage_btf_ids,

};



const struct bpf_func_proto bpf_inode_storage_delete_proto = {
@@ -270,5 +268,5 @@ const struct bpf_func_proto bpf_inode_storage_delete_proto = { 
	.ret_type	= RET_INTEGER,

	.arg1_type	= ARG_CONST_MAP_PTR,

	.arg2_type	= ARG_PTR_TO_BTF_ID,

	.btf_id		= bpf_inode_storage_btf_ids,

	.arg2_btf_id	= &bpf_inode_storage_btf_ids[0],

};

kernel/bpf/btf.c

+1 −14
Original line number Diff line number Diff line
@@ -4193,19 +4193,6 @@ bool btf_struct_ids_match(struct bpf_verifier_log *log, 
	return true;

}



int btf_resolve_helper_id(struct bpf_verifier_log *log,

			  const struct bpf_func_proto *fn, int arg)

{

	int id;



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

		return -EINVAL;

	id = fn->btf_id[arg];

	if (!id || id > btf_vmlinux->nr_types)

		return -EINVAL;

	return id;

}



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

			   const struct btf_type **bad_type)

{
@@ -4772,7 +4759,7 @@ static int btf_id_cmp_func(const void *a, const void *b) 
	return *pa - *pb;

}



bool btf_id_set_contains(struct btf_id_set *set, u32 id)

bool btf_id_set_contains(const struct btf_id_set *set, u32 id)

{

	return bsearch(&id, set->ids, set->cnt, sizeof(u32), btf_id_cmp_func) != NULL;

}

kernel/bpf/stackmap.c

+2 −3
Original line number Diff line number Diff line
@@ -665,18 +665,17 @@ BPF_CALL_4(bpf_get_task_stack, struct task_struct *, task, void *, buf, 
	return __bpf_get_stack(regs, task, NULL, buf, size, flags);

}



BTF_ID_LIST(bpf_get_task_stack_btf_ids)

BTF_ID(struct, task_struct)

BTF_ID_LIST_SINGLE(bpf_get_task_stack_btf_ids, struct, task_struct)



const struct bpf_func_proto bpf_get_task_stack_proto = {

	.func		= bpf_get_task_stack,

	.gpl_only	= false,

	.ret_type	= RET_INTEGER,

	.arg1_type	= ARG_PTR_TO_BTF_ID,

	.arg1_btf_id	= &bpf_get_task_stack_btf_ids[0],

	.arg2_type	= ARG_PTR_TO_UNINIT_MEM,

	.arg3_type	= ARG_CONST_SIZE_OR_ZERO,

	.arg4_type	= ARG_ANYTHING,

	.btf_id		= bpf_get_task_stack_btf_ids,

};



BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,