History

bpf: Add abnormal return checks.

LD_[ABS|IND] instructions may return from the function early. bpf_tail_call pseudo instruction is either fallthrough or return. Allow them in the subprograms only when subprograms are BTF annotated and have scalar return types. Allow ld_abs and tail_call in the main program even if it calls into subprograms. In the past that was not ok to do for ld_abs, since it was JITed with special exit sequence. Since bpf_gen_ld_abs() was introduced the ld_abs looks like normal exit insn from JIT point of view, so it's safe to allow them in the main program.

Alexei Starovoitov committed 6 years ago

Commit 09b28d71 parent e4119013 files · +52 −22Browse files

Unified · Split

3 files
  1. include/linux/bpf_verifier.h +1 −0
  2. kernel/bpf/verifier.c +48 −19
  3. tools/testing/selftests/bpf/verifier/calls.c +3 −3
include/linux/bpf_verifier.h+1−0
@@ -360,6 +360,7 @@ struct bpf_subprog_info { · 359 unmodified lines
360360 u16 stack_depth; /* max. stack depth used by this function */
361361 bool has_tail_call;
362362 bool tail_call_reachable;
363+ bool has_ld_abs;
363364 };
364365
365366 /* single container for all structs
kernel/bpf/verifier.c+48−19
@@ -1494,6 +1494,9 @@ static int check_subprogs(struct bpf_verifier_env *env) · 1493 unmodified lines
14941494 insn[i].imm == BPF_FUNC_tail_call &&
14951495 insn[i].src_reg != BPF_PSEUDO_CALL)
14961496 subprog[cur_subprog].has_tail_call = true;
1497+ if (BPF_CLASS(code) == BPF_LD &&
1498+ (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
1499+ subprog[cur_subprog].has_ld_abs = true;
14971500 if (BPF_CLASS(code) != BPF_JMP && BPF_CLASS(code) != BPF_JMP32)
14981501 goto next;
14991502 if (BPF_OP(code) == BPF_EXIT || BPF_OP(code) == BPF_CALL)
@@ -7514,18 +7517,6 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn) · 6014 unmodified lines
75147517 return -EINVAL;
75157518 }
75167519
7517− if (env->subprog_cnt > 1) {
7518− /* when program has LD_ABS insn JITs and interpreter assume
7519− * that r1 == ctx == skb which is not the case for callees
7520− * that can have arbitrary arguments. It's problematic
7521− * for main prog as well since JITs would need to analyze
7522− * all functions in order to make proper register save/restore
7523− * decisions in the main prog. Hence disallow LD_ABS with calls
7524− */
7525− verbose(env, "BPF_LD_[ABS|IND] instructions cannot be mixed with bpf-to-bpf calls\n");
7526− return -EINVAL;
7527− }
7528−
75297520 if (insn->dst_reg != BPF_REG_0 || insn->off != 0 ||
75307521 BPF_SIZE(insn->code) == BPF_DW ||
75317522 (mode == BPF_ABS && insn->src_reg != BPF_REG_0)) {
@@ -7936,6 +7927,23 @@ err_free: · 404 unmodified lines
79367927 return ret;
79377928 }
79387929
7930+static int check_abnormal_return(struct bpf_verifier_env *env)
7931+{
7932+ int i;
7933+
7934+ for (i = 1; i < env->subprog_cnt; i++) {
7935+ if (env->subprog_info[i].has_ld_abs) {
7936+ verbose(env, "LD_ABS is not allowed in subprogs without BTF\n");
7937+ return -EINVAL;
7938+ }
7939+ if (env->subprog_info[i].has_tail_call) {
7940+ verbose(env, "tail_call is not allowed in subprogs without BTF\n");
7941+ return -EINVAL;
7942+ }
7943+ }
7944+ return 0;
7945+}
7946+
79397947 /* The minimum supported BTF func info size */
79407948 #define MIN_BPF_FUNCINFO_SIZE 8
79417949 #define MAX_FUNCINFO_REC_SIZE 252
@@ -7944,20 +7952,24 @@ static int check_btf_func(struct bpf_verifier_env *env, · 2 unmodified lines
79447952 const union bpf_attr *attr,
79457953 union bpf_attr __user *uattr)
79467954 {
7955+ const struct btf_type *type, *func_proto, *ret_type;
79477956 u32 i, nfuncs, urec_size, min_size;
79487957 u32 krec_size = sizeof(struct bpf_func_info);
79497958 struct bpf_func_info *krecord;
79507959 struct bpf_func_info_aux *info_aux = NULL;
7951− const struct btf_type *type;
79527960 struct bpf_prog *prog;
79537961 const struct btf *btf;
79547962 void __user *urecord;
79557963 u32 prev_offset = 0;
7964+ bool scalar_return;
79567965 int ret = -ENOMEM;
79577966
79587967 nfuncs = attr->func_info_cnt;
7959− if (!nfuncs)
7968+ if (!nfuncs) {
7969+ if (check_abnormal_return(env))
7970+ return -EINVAL;
79607971 return 0;
7972+ }
79617973
79627974 if (nfuncs != env->subprog_cnt) {
79637975 verbose(env, "number of funcs in func_info doesn't match number of subprogs\n");
@@ -8005,25 +8017,23 @@ static int check_btf_func(struct bpf_verifier_env *env, · 41 unmodified lines
80058017 }
80068018
80078019 /* check insn_off */
8020+ ret = -EINVAL;
80088021 if (i == 0) {
80098022 if (krecord[i].insn_off) {
80108023 verbose(env,
80118024 "nonzero insn_off %u for the first func info record",
80128025 krecord[i].insn_off);
8013− ret = -EINVAL;
80148026 goto err_free;
80158027 }
80168028 } else if (krecord[i].insn_off <= prev_offset) {
80178029 verbose(env,
80188030 "same or smaller insn offset (%u) than previous func info record (%u)",
80198031 krecord[i].insn_off, prev_offset);
8020− ret = -EINVAL;
80218032 goto err_free;
80228033 }
80238034
80248035 if (env->subprog_info[i].start != krecord[i].insn_off) {
80258036 verbose(env, "func_info BTF section doesn't match subprog layout in BPF program\n");
8026− ret = -EINVAL;
80278037 goto err_free;
80288038 }
80298039
@@ -8032,10 +8042,26 @@ static int check_btf_func(struct bpf_verifier_env *env, · 2 unmodified lines
80328042 if (!type || !btf_type_is_func(type)) {
80338043 verbose(env, "invalid type id %d in func info",
80348044 krecord[i].type_id);
8035− ret = -EINVAL;
80368045 goto err_free;
80378046 }
80388047 info_aux[i].linkage = BTF_INFO_VLEN(type->info);
8048+
8049+ func_proto = btf_type_by_id(btf, type->type);
8050+ if (unlikely(!func_proto || !btf_type_is_func_proto(func_proto)))
8051+ /* btf_func_check() already verified it during BTF load */
8052+ goto err_free;
8053+ ret_type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
8054+ scalar_return =
8055+ btf_type_is_small_int(ret_type) || btf_type_is_enum(ret_type);
8056+ if (i && !scalar_return && env->subprog_info[i].has_ld_abs) {
8057+ verbose(env, "LD_ABS is only allowed in functions that return 'int'.\n");
8058+ goto err_free;
8059+ }
8060+ if (i && !scalar_return && env->subprog_info[i].has_tail_call) {
8061+ verbose(env, "tail_call is only allowed in functions that return 'int'.\n");
8062+ goto err_free;
8063+ }
8064+
80398065 prev_offset = krecord[i].insn_off;
80408066 urecord += urec_size;
80418067 }
@@ -8196,8 +8222,11 @@ static int check_btf_info(struct bpf_verifier_env *env, · 154 unmodified lines
81968222 struct btf *btf;
81978223 int err;
81988224
8199− if (!attr->func_info_cnt && !attr->line_info_cnt)
8225+ if (!attr->func_info_cnt && !attr->line_info_cnt) {
8226+ if (check_abnormal_return(env))
8227+ return -EINVAL;
82008228 return 0;
8229+ }
82018230
82028231 btf = btf_get_by_fd(attr->prog_btf_fd);
82038232 if (IS_ERR(btf))
tools/testing/selftests/bpf/verifier/calls.c+3−3
@@ -647,13 +647,14 @@ · 646 unmodified lines
647647 .result = REJECT,
648648 },
649649 {
650− "calls: ld_abs with changing ctx data in callee",
650+ "calls: subprog call with ld_abs in main prog",
651651 .insns = {
652652 BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
653653 BPF_LD_ABS(BPF_B, 0),
654654 BPF_LD_ABS(BPF_H, 0),
655655 BPF_LD_ABS(BPF_W, 0),
656656 BPF_MOV64_REG(BPF_REG_7, BPF_REG_6),
657+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
657658 BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 5),
658659 BPF_MOV64_REG(BPF_REG_6, BPF_REG_7),
659660 BPF_LD_ABS(BPF_B, 0),
@@ -666,8 +667,7 @@ · 6 unmodified lines
666667 BPF_EXIT_INSN(),
667668 },
668669 .prog_type = BPF_PROG_TYPE_SCHED_CLS,
669− .errstr = "BPF_LD_[ABS|IND] instructions cannot be mixed",
670− .result = REJECT,
670+ .result = ACCEPT,
671671 },
672672 {
673673 "calls: two calls with bad fallthrough",

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t