bpf: fix branch pruning logic

when the verifier detects that register contains a runtime constant and it's compared with another constant it will prune exploration of the branch that is guaranteed not to be taken at runtime. This is all correct, but malicious program may be constructed in such a way that it always has a constant comparison and the other branch is never taken under any conditions. In this case such path through the program will not be explored by the verifier. It won't be taken at run-time either, but since all instructions are JITed the malicious program may cause JITs to complain about using reserved fields, etc. To fix the issue we have to track the instructions explored by the verifier and sanitize instructions that are dead at run time with NOPs. We cannot reject such dead code, since llvm generates it for valid C code, since it doesn't do as much data flow analysis as the verifier does.

Alexei Starovoitov committed 8 years ago

Commit c1311871 parent 107af8e2 files · +28 −1Browse files

Unified · Split

2 files

include/linux/bpf_verifier.h +1 −1
kernel/bpf/verifier.c +27 −0

include/linux/bpf_verifier.h+1−1

@@ -115,7 +115,7 @@ struct bpf_insn_aux_data { · 114 unmodified lines
115	115	struct bpf_map map_ptr; / pointer for call insn into lookup_elem */
116	116	};
117	117	int ctx_field_size; /* the ctx field size for load insn, maybe 0 */
118		− int converted_op_size; /* the valid value width after perceived conversion */
	118	+ bool seen; /* this insn was processed by the verifier */
119	119	};
120	120
121	121	#define MAX_USED_MAPS 64 /* max number of maps accessed by one eBPF program */

kernel/bpf/verifier.c+27−0

@@ -3827,6 +3827,7 @@ static int do_check(struct bpf_verifier_env *env) · 3826 unmodified lines
3827	3827	return err;
3828	3828
3829	3829	regs = cur_regs(env);
	3830	+ env->insn_aux_data[insn_idx].seen = true;
3830	3831	if (class == BPF_ALU \|\| class == BPF_ALU64) {
3831	3832	err = check_alu_op(env, insn);
3832	3833	if (err)
@@ -4022,6 +4023,7 @@ process_bpf_exit: · 189 unmodified lines
4022	4023	return err;
4023	4024
4024	4025	insn_idx++;
	4026	+ env->insn_aux_data[insn_idx].seen = true;
4025	4027	} else {
4026	4028	verbose(env, "invalid BPF_LD mode\n");
4027	4029	return -EINVAL;
@@ -4204,6 +4206,7 @@ static int adjust_insn_aux_data(struct bpf_verifier_env *env, u32 prog_len, · 176 unmodified lines
4204	4206	u32 off, u32 cnt)
4205	4207	{
4206	4208	struct bpf_insn_aux_data new_data, old_data = env->insn_aux_data;
	4209	+ int i;
4207	4210
4208	4211	if (cnt == 1)
4209	4212	return 0;
@@ -4213,6 +4216,8 @@ static int adjust_insn_aux_data(struct bpf_verifier_env *env, u32 prog_len, · 3 unmodified lines
4213	4216	memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off);
4214	4217	memcpy(new_data + off + cnt - 1, old_data + off,
4215	4218	sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
	4219	+ for (i = off; i < off + cnt - 1; i++)
	4220	+ new_data[i].seen = true;
4216	4221	env->insn_aux_data = new_data;
4217	4222	vfree(old_data);
4218	4223	return 0;
@@ -4231,6 +4236,25 @@ static struct bpf_prog bpf_patch_insn_data(struct bpf_verifier_env env, u32 of · 12 unmodified lines
4231	4236	return new_prog;
4232	4237	}
4233	4238
	4239	+/* The verifier does more data flow analysis than llvm and will not explore
	4240	+ * branches that are dead at run time. Malicious programs can have dead code
	4241	+ * too. Therefore replace all dead at-run-time code with nops.
	4242	+ */
	4243	+static void sanitize_dead_code(struct bpf_verifier_env *env)
	4244	+{
	4245	+ struct bpf_insn_aux_data *aux_data = env->insn_aux_data;
	4246	+ struct bpf_insn nop = BPF_MOV64_REG(BPF_REG_0, BPF_REG_0);
	4247	+ struct bpf_insn *insn = env->prog->insnsi;
	4248	+ const int insn_cnt = env->prog->len;
	4249	+ int i;
	4250	+
	4251	+ for (i = 0; i < insn_cnt; i++) {
	4252	+ if (aux_data[i].seen)
	4253	+ continue;
	4254	+ memcpy(insn + i, &nop, sizeof(nop));
	4255	+ }
	4256	+}
	4257	+
4234	4258	/* convert load instructions that access fields of 'struct __sk_buff'
4235	4259	* into sequence of instructions that access fields of 'struct sk_buff'
4236	4260	*/
@@ -4557,6 +4581,9 @@ skip_full_check: · 320 unmodified lines
4557	4581	while (!pop_stack(env, NULL, NULL));
4558	4582	free_states(env);
4559	4583
	4584	+ if (ret == 0)
	4585	+ sanitize_dead_code(env);
	4586	+
4560	4587	if (ret == 0)
4561	4588	/* program is valid, convert (u32)(ctx + off) accesses */
4562	4589	ret = convert_ctx_accesses(env);