Skip to content

Commit

Permalink
bpf: Allow specifying a BTF ID per argument in function protos
Browse files Browse the repository at this point in the history
Function prototypes using ARG_PTR_TO_BTF_ID currently use two ways to signal
which BTF IDs are acceptable. First, bpf_func_proto.btf_id is an array of
IDs, one for each argument. This array is only accessed up to the highest
numbered argument that uses ARG_PTR_TO_BTF_ID and may therefore be less than
five arguments long. It usually points at a BTF_ID_LIST. Second, check_btf_id
is a function pointer that is called by the verifier if present. It gets the
actual BTF ID of the register, and the argument number we're currently checking.
It turns out that the only user check_arg_btf_id ignores the argument, and is
simply used to check whether the BTF ID has a struct sock_common at it's start.

Replace both of these mechanisms with an explicit BTF ID for each argument
in a function proto. Thanks to btf_struct_ids_match this is very flexible:
check_arg_btf_id can be replaced by requiring struct sock_common.

Signed-off-by: Lorenz Bauer <[email protected]>
Signed-off-by: Alexei Starovoitov <[email protected]>
Acked-by: Martin KaFai Lau <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
  • Loading branch information
lmb authored and Alexei Starovoitov committed Sep 21, 2020
1 parent 27774b7 commit 9436ef6
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 103 deletions.
18 changes: 10 additions & 8 deletions include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,12 +326,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);
};
Expand Down Expand Up @@ -1385,8 +1389,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,
Expand Down
8 changes: 3 additions & 5 deletions kernel/bpf/bpf_inode_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -249,19 +249,17 @@ 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,
.gpl_only = false,
.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 = {
Expand All @@ -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],
};
13 changes: 0 additions & 13 deletions kernel/bpf/btf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
5 changes: 2 additions & 3 deletions kernel/bpf/stackmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
44 changes: 22 additions & 22 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ struct bpf_call_arg_meta {
u64 msize_max_value;
int ref_obj_id;
int func_id;
u32 btf_id;
};

struct btf *btf_vmlinux;
Expand Down Expand Up @@ -4049,29 +4048,23 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
goto err_type;
}
} else if (arg_type == ARG_PTR_TO_BTF_ID) {
bool ids_match = false;
const u32 *btf_id = fn->arg_btf_id[arg];

expected_type = PTR_TO_BTF_ID;
if (type != expected_type)
goto err_type;
if (!fn->check_btf_id) {
if (reg->btf_id != meta->btf_id) {
ids_match = btf_struct_ids_match(&env->log, reg->off, reg->btf_id,
meta->btf_id);
if (!ids_match) {
verbose(env, "Helper has type %s got %s in R%d\n",
kernel_type_name(meta->btf_id),
kernel_type_name(reg->btf_id), regno);
return -EACCES;
}
}
} else if (!fn->check_btf_id(reg->btf_id, arg)) {
verbose(env, "Helper does not support %s in R%d\n",
kernel_type_name(reg->btf_id), regno);

if (!btf_id) {
verbose(env, "verifier internal error: missing BTF ID\n");
return -EFAULT;
}

if (!btf_struct_ids_match(&env->log, reg->off, reg->btf_id, *btf_id)) {
verbose(env, "R%d is of type %s but %s is expected\n",
regno, kernel_type_name(reg->btf_id), kernel_type_name(*btf_id));
return -EACCES;
}
if ((reg->off && !ids_match) || !tnum_is_const(reg->var_off) || reg->var_off.value) {
if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
verbose(env, "R%d is a pointer to in-kernel struct with non-zero offset\n",
regno);
return -EACCES;
Expand Down Expand Up @@ -4545,10 +4538,22 @@ static bool check_refcount_ok(const struct bpf_func_proto *fn, int func_id)
return count <= 1;
}

static bool check_btf_id_ok(const struct bpf_func_proto *fn)
{
int i;

for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++)
if (fn->arg_type[i] == ARG_PTR_TO_BTF_ID && !fn->arg_btf_id[i])
return false;

return true;
}

static int check_func_proto(const struct bpf_func_proto *fn, int func_id)
{
return check_raw_mode_ok(fn) &&
check_arg_pair_ok(fn) &&
check_btf_id_ok(fn) &&
check_refcount_ok(fn, func_id) ? 0 : -EINVAL;
}

Expand Down Expand Up @@ -4944,11 +4949,6 @@ 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->check_btf_id) {
err = btf_resolve_helper_id(&env->log, fn, i);
if (err > 0)
meta.btf_id = err;
}
err = check_func_arg(env, i, &meta, fn);
if (err)
return err;
Expand Down
15 changes: 5 additions & 10 deletions kernel/trace/bpf_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -743,37 +743,33 @@ BPF_CALL_5(bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
return err;
}

BTF_ID_LIST(bpf_seq_printf_btf_ids)
BTF_ID(struct, seq_file)
BTF_ID_LIST_SINGLE(btf_seq_file_ids, struct, seq_file)

static const struct bpf_func_proto bpf_seq_printf_proto = {
.func = bpf_seq_printf,
.gpl_only = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_BTF_ID,
.arg1_btf_id = &btf_seq_file_ids[0],
.arg2_type = ARG_PTR_TO_MEM,
.arg3_type = ARG_CONST_SIZE,
.arg4_type = ARG_PTR_TO_MEM_OR_NULL,
.arg5_type = ARG_CONST_SIZE_OR_ZERO,
.btf_id = bpf_seq_printf_btf_ids,
};

BPF_CALL_3(bpf_seq_write, struct seq_file *, m, const void *, data, u32, len)
{
return seq_write(m, data, len) ? -EOVERFLOW : 0;
}

BTF_ID_LIST(bpf_seq_write_btf_ids)
BTF_ID(struct, seq_file)

static const struct bpf_func_proto bpf_seq_write_proto = {
.func = bpf_seq_write,
.gpl_only = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_BTF_ID,
.arg1_btf_id = &btf_seq_file_ids[0],
.arg2_type = ARG_PTR_TO_MEM,
.arg3_type = ARG_CONST_SIZE_OR_ZERO,
.btf_id = bpf_seq_write_btf_ids,
};

static __always_inline int
Expand Down Expand Up @@ -1130,17 +1126,16 @@ static bool bpf_d_path_allowed(const struct bpf_prog *prog)
return btf_id_set_contains(&btf_allowlist_d_path, prog->aux->attach_btf_id);
}

BTF_ID_LIST(bpf_d_path_btf_ids)
BTF_ID(struct, path)
BTF_ID_LIST_SINGLE(bpf_d_path_btf_ids, struct, path)

static const struct bpf_func_proto bpf_d_path_proto = {
.func = bpf_d_path,
.gpl_only = false,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_BTF_ID,
.arg1_btf_id = &bpf_d_path_btf_ids[0],
.arg2_type = ARG_PTR_TO_MEM,
.arg3_type = ARG_CONST_SIZE_OR_ZERO,
.btf_id = bpf_d_path_btf_ids,
.allowed = bpf_d_path_allowed,
};

Expand Down
8 changes: 2 additions & 6 deletions net/core/bpf_sk_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -378,19 +378,15 @@ const struct bpf_func_proto bpf_sk_storage_delete_proto = {
.arg2_type = ARG_PTR_TO_SOCKET,
};

BTF_ID_LIST(sk_storage_btf_ids)
BTF_ID_UNUSED
BTF_ID(struct, sock)

const struct bpf_func_proto sk_storage_get_btf_proto = {
.func = bpf_sk_storage_get,
.gpl_only = false,
.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 = &btf_sock_ids[BTF_SOCK_TYPE_SOCK],
.arg3_type = ARG_PTR_TO_MAP_VALUE_OR_NULL,
.arg4_type = ARG_ANYTHING,
.btf_id = sk_storage_btf_ids,
};

const struct bpf_func_proto sk_storage_delete_btf_proto = {
Expand All @@ -399,7 +395,7 @@ const struct bpf_func_proto sk_storage_delete_btf_proto = {
.ret_type = RET_INTEGER,
.arg1_type = ARG_CONST_MAP_PTR,
.arg2_type = ARG_PTR_TO_BTF_ID,
.btf_id = sk_storage_btf_ids,
.arg2_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_SOCK],
};

struct bpf_sk_storage_diag {
Expand Down
31 changes: 9 additions & 22 deletions net/core/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -3803,19 +3803,18 @@ static const struct bpf_func_proto bpf_skb_event_output_proto = {
.arg5_type = ARG_CONST_SIZE_OR_ZERO,
};

BTF_ID_LIST(bpf_skb_output_btf_ids)
BTF_ID(struct, sk_buff)
BTF_ID_LIST_SINGLE(bpf_skb_output_btf_ids, struct, sk_buff)

const struct bpf_func_proto bpf_skb_output_proto = {
.func = bpf_skb_event_output,
.gpl_only = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_BTF_ID,
.arg1_btf_id = &bpf_skb_output_btf_ids[0],
.arg2_type = ARG_CONST_MAP_PTR,
.arg3_type = ARG_ANYTHING,
.arg4_type = ARG_PTR_TO_MEM,
.arg5_type = ARG_CONST_SIZE_OR_ZERO,
.btf_id = bpf_skb_output_btf_ids,
};

static unsigned short bpf_tunnel_key_af(u64 flags)
Expand Down Expand Up @@ -4199,19 +4198,18 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
.arg5_type = ARG_CONST_SIZE_OR_ZERO,
};

BTF_ID_LIST(bpf_xdp_output_btf_ids)
BTF_ID(struct, xdp_buff)
BTF_ID_LIST_SINGLE(bpf_xdp_output_btf_ids, struct, xdp_buff)

const struct bpf_func_proto bpf_xdp_output_proto = {
.func = bpf_xdp_event_output,
.gpl_only = true,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_BTF_ID,
.arg1_btf_id = &bpf_xdp_output_btf_ids[0],
.arg2_type = ARG_CONST_MAP_PTR,
.arg3_type = ARG_ANYTHING,
.arg4_type = ARG_PTR_TO_MEM,
.arg5_type = ARG_CONST_SIZE_OR_ZERO,
.btf_id = bpf_xdp_output_btf_ids,
};

BPF_CALL_1(bpf_get_socket_cookie, struct sk_buff *, skb)
Expand Down Expand Up @@ -9897,17 +9895,6 @@ BTF_SOCK_TYPE_xxx
u32 btf_sock_ids[MAX_BTF_SOCK_TYPE];
#endif

static bool check_arg_btf_id(u32 btf_id, u32 arg)
{
int i;

/* only one argument, no need to check arg */
for (i = 0; i < MAX_BTF_SOCK_TYPE; i++)
if (btf_sock_ids[i] == btf_id)
return true;
return false;
}

BPF_CALL_1(bpf_skc_to_tcp6_sock, struct sock *, sk)
{
/* tcp6_sock type is not generated in dwarf and hence btf,
Expand All @@ -9926,7 +9913,7 @@ const struct bpf_func_proto bpf_skc_to_tcp6_sock_proto = {
.gpl_only = false,
.ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
.arg1_type = ARG_PTR_TO_BTF_ID,
.check_btf_id = check_arg_btf_id,
.arg1_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_SOCK_COMMON],
.ret_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_TCP6],
};

Expand All @@ -9943,7 +9930,7 @@ const struct bpf_func_proto bpf_skc_to_tcp_sock_proto = {
.gpl_only = false,
.ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
.arg1_type = ARG_PTR_TO_BTF_ID,
.check_btf_id = check_arg_btf_id,
.arg1_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_SOCK_COMMON],
.ret_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_TCP],
};

Expand All @@ -9967,7 +9954,7 @@ const struct bpf_func_proto bpf_skc_to_tcp_timewait_sock_proto = {
.gpl_only = false,
.ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
.arg1_type = ARG_PTR_TO_BTF_ID,
.check_btf_id = check_arg_btf_id,
.arg1_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_SOCK_COMMON],
.ret_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_TCP_TW],
};

Expand All @@ -9991,7 +9978,7 @@ const struct bpf_func_proto bpf_skc_to_tcp_request_sock_proto = {
.gpl_only = false,
.ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
.arg1_type = ARG_PTR_TO_BTF_ID,
.check_btf_id = check_arg_btf_id,
.arg1_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_SOCK_COMMON],
.ret_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_TCP_REQ],
};

Expand All @@ -10013,6 +10000,6 @@ const struct bpf_func_proto bpf_skc_to_udp6_sock_proto = {
.gpl_only = false,
.ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
.arg1_type = ARG_PTR_TO_BTF_ID,
.check_btf_id = check_arg_btf_id,
.arg1_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_SOCK_COMMON],
.ret_btf_id = &btf_sock_ids[BTF_SOCK_TYPE_UDP6],
};
Loading

0 comments on commit 9436ef6

Please sign in to comment.