bpf: handle ldimm64 properly in check_cfg()

ldimm64 instructions are 16-byte long, and so have to be handled appropriately in check_cfg(), just like the rest of BPF verifier does. This has implications in three places: - when determining next instruction for non-jump instructions; - when determining next instruction for callback address ldimm64 instructions (in visit_func_call_insn()); - when checking for unreachable instructions, where second half of ldimm64 is expected to be unreachable; We take this also as an opportunity to report jump into the middle of ldimm64. And adjust few test_verifier tests accordingly.

Andrii Nakryiko committed 3 years ago

Commit 3feb2631 parent fe69a1b3 files · +30 −13Browse files

Unified · Split

3 files

include/linux/bpf.h +6 −2
kernel/bpf/verifier.c +20 −7
tools/testing/selftests/bpf/verifier/ld_imm64.c +4 −4

include/linux/bpf.h+6−2

 	aux->ctx_field_size = size;
 }
+static bool bpf_is_ldimm64(const struct bpf_insn *insn)
+{
+	return insn->code == (BPF_LD | BPF_IMM | BPF_DW);
+}
 static inline bool bpf_pseudo_func(const struct bpf_insn *insn)
 {
-	return insn->code == (BPF_LD | BPF_IMM | BPF_DW) &&
+	return bpf_is_ldimm64(insn) && insn->src_reg == BPF_PSEUDO_FUNC;
-	       insn->src_reg == BPF_PSEUDO_FUNC;
 }
 struct bpf_prog_ops {

kernel/bpf/verifier.c+20−7

 				struct bpf_verifier_env *env,
 				bool visit_callee)
 {
-	int ret;
+	int ret, insn_sz;
-	ret = push_insn(t, t + 1, FALLTHROUGH, env, false);
+	insn_sz = bpf_is_ldimm64(&insns[t]) ? 2 : 1;
+	ret = push_insn(t, t + insn_sz, FALLTHROUGH, env, false);
 	if (ret)
 		return ret;
-	mark_prune_point(env, t + 1);
+	mark_prune_point(env, t + insn_sz);
 	/* when we exit from subprog, we need to record non-linear history */
-	mark_jmp_point(env, t + 1);
+	mark_jmp_point(env, t + insn_sz);
 	if (visit_callee) {
 		mark_prune_point(env, t);
 static int visit_insn(int t, struct bpf_verifier_env *env)
 {
 	struct bpf_insn *insns = env->prog->insnsi, *insn = &insns[t];
-	int ret, off;
+	int ret, off, insn_sz;
 	if (bpf_pseudo_func(insn))
 		return visit_func_call_insn(t, insns, env, true);
 	/* All non-branch instructions have a single fall-through edge. */
 	if (BPF_CLASS(insn->code) != BPF_JMP &&
-	    BPF_CLASS(insn->code) != BPF_JMP32)
+	    BPF_CLASS(insn->code) != BPF_JMP32) {
-		return push_insn(t, t + 1, FALLTHROUGH, env, false);
+		insn_sz = bpf_is_ldimm64(insn) ? 2 : 1;
+		return push_insn(t, t + insn_sz, FALLTHROUGH, env, false);
+	}
 	switch (BPF_OP(insn->code)) {
 	case BPF_EXIT:
 	}
 	for (i = 0; i < insn_cnt; i++) {
+		struct bpf_insn *insn = &env->prog->insnsi[i];
 		if (insn_state[i] != EXPLORED) {
 			verbose(env, "unreachable insn %d\n", i);
 			ret = -EINVAL;
 			goto err_free;
 		}
+		if (bpf_is_ldimm64(insn)) {
+			if (insn_state[i + 1] != 0) {
+				verbose(env, "jump into the middle of ldimm64 insn %d\n", i);
+				ret = -EINVAL;
+				goto err_free;
+			}
+			i++; /* skip second half of ldimm64 */
+		}
 	}
 	ret = 0; /* cfg looks good */

tools/testing/selftests/bpf/verifier/ld_imm64.c+4−4

@@ -9,8 +9,8 @@ · 8 unmodified lines
9	BPF_MOV64_IMM(BPF_REG_0, 2),	9	BPF_MOV64_IMM(BPF_REG_0, 2),
10	BPF_EXIT_INSN(),	10	BPF_EXIT_INSN(),
11	},	11	},
12	.errstr = "invalid BPF_LD_IMM insn",	12	.errstr = "jump into the middle of ldimm64 insn 1",
13	.errstr_unpriv = "R1 pointer comparison",	13	.errstr_unpriv = "jump into the middle of ldimm64 insn 1",
14	.result = REJECT,	14	.result = REJECT,
15	},	15	},
16	{	16	{
@@ -23,8 +23,8 @@ · 6 unmodified lines
23	BPF_LD_IMM64(BPF_REG_0, 1),	23	BPF_LD_IMM64(BPF_REG_0, 1),
24	BPF_EXIT_INSN(),	24	BPF_EXIT_INSN(),
25	},	25	},
26	.errstr = "invalid BPF_LD_IMM insn",	26	.errstr = "jump into the middle of ldimm64 insn 1",
27	.errstr_unpriv = "R1 pointer comparison",	27	.errstr_unpriv = "jump into the middle of ldimm64 insn 1",
28	.result = REJECT,	28	.result = REJECT,
29	},	29	},
30	{	30	{