bpf: Rewrite any fault prone load out of a mem or btf_id pointer

bpf_convert_ctx_accesses() turns a BPF_LDX into a BPF_PROBE_MEM one by matching the type recorded for the insn against a list of exact pointer types. The list cannot keep up with the flag combinations the verifier produces, and a type which is missing from it ends up as a plain load without an exception table entry, so a bad address panics the kernel instead of being handled. Two such types exist today and are reachable: - PTR_TO_BTF_ID | PTR_UNTRUSTED | MEM_ALLOC | NON_OWN_REF - PTR_TO_BTF_ID | PTR_UNTRUSTED | MEM_RCU Rather than adding the two, just drop the list and state the property itself in the default case of the switch. This is a superset of what the list matched, the untrusted PTR_TO_MEM does not have to carry MEM_RDONLY for it anymore, and it stays in sync with the verifier side which uses the same match in save_aux_ptr_type() and reg_type_mismatch_ok(). Assert that a fault prone type which does not get the rewrite for whatever reason is rejected at load time rather than left to fault at runtime to catch any future cases.

Daniel Borkmann committed 3 months ago

Commit d99bda71 parent ee9ad133 files · +39 −34Browse files

Unified · Split

3 files

include/linux/bpf_verifier.h +11 −0
kernel/bpf/fixups.c +26 −21
kernel/bpf/verifier.c +2 −13

include/linux/bpf_verifier.h+11−0

@@ -1308,6 +1308,17 @@ static inline u32 type_flag(u32 type) · 1307 unmodified lines
1308	return type & ~BPF_BASE_TYPE_MASK;	1308	return type & ~BPF_BASE_TYPE_MASK;
1309	}	1309	}
1310		1310
		1311	static inline bool bpf_is_ptr_to_mem_or_btf_id(enum bpf_reg_type type)
		1312	{
		1313	switch (base_type(type)) {
		1314	case PTR_TO_MEM:
		1315	case PTR_TO_BTF_ID:
		1316	return true;
		1317	default:
		1318	return false;
		1319	}
		1320	}
		1321
1311	static inline bool bpf_may_fault_on_deref(enum bpf_reg_type type)	1322	static inline bool bpf_may_fault_on_deref(enum bpf_reg_type type)
1312	{	1323	{
1313	/*	1324	/*

kernel/bpf/fixups.c+26−21

 	for (i = 0; i < insn_cnt; i++, insn++) {
 		bpf_convert_ctx_access_t convert_ctx_access;
+		enum bpf_reg_type ptr_type;
 		u8 mode;
 		if (env->insn_aux_data[i + delta].nospec) {
 			continue;
 		}
-		switch ((int)env->insn_aux_data[i + delta].ptr_type) {
+		ptr_type = env->insn_aux_data[i + delta].ptr_type;
+		switch ((int)ptr_type) {
 		case PTR_TO_CTX:
 			if (!ops->convert_ctx_access)
 				continue;
 		case PTR_TO_XDP_SOCK:
 			convert_ctx_access = bpf_xdp_sock_convert_ctx_access;
 			break;
-		case PTR_TO_BTF_ID:
-		case PTR_TO_BTF_ID | PTR_UNTRUSTED:
-		/* PTR_TO_BTF_ID | MEM_ALLOC always has a valid lifetime, unlike
-		 * PTR_TO_BTF_ID, and an active referenced id, but the same cannot
-		 * be said once it is marked PTR_UNTRUSTED, hence we must handle
-		 * any faults for loads into such types. BPF_WRITE is disallowed
-		 * for this case.
-		 */
-		case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
-		case PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED:
-			if (type == BPF_READ) {
-				if (BPF_MODE(insn->code) == BPF_MEM)
-					insn->code = BPF_LDX | BPF_PROBE_MEM |
-						     BPF_SIZE((insn)->code);
-				else
-					insn->code = BPF_LDX | BPF_PROBE_MEMSX |
-						     BPF_SIZE((insn)->code);
-				env->prog->aux->num_exentries++;
-			}
-			continue;
 		case PTR_TO_ARENA:
 			if (BPF_MODE(insn->code) == BPF_MEMSX) {
 				if (!bpf_jit_supports_insn(insn, true)) {
 			env->prog->aux->num_exentries++;
 			continue;
 		default:
+			/*
+			 * A pointer which may fault on a dereference must not
+			 * be loaded from without fault protection, hence turn
+			 * the BPF_LDX into a BPF_PROBE_MEM one so that a bad
+			 * address is handled rather than panicking the kernel.
+			 * A store through one is rejected earlier, there is no
+			 * probed counterpart to rewrite it into.
+			 */
+			if (bpf_is_ptr_to_mem_or_btf_id(ptr_type) &&
+			    bpf_may_fault_on_deref(ptr_type) &&
+			    type == BPF_READ) {
+				if (BPF_MODE(insn->code) == BPF_MEM)
+					insn->code = BPF_LDX | BPF_PROBE_MEM |
+						     BPF_SIZE(insn->code);
+				else
+					insn->code = BPF_LDX | BPF_PROBE_MEMSX |
+						     BPF_SIZE(insn->code);
+				env->prog->aux->num_exentries++;
+				continue;
+			}
+			if (verifier_bug_if(bpf_may_fault_on_deref(ptr_type), env,
+					    "access to a fault prone pointer is not rewritten as a probed one"))
+				return -EFAULT;
 			continue;
 		}

kernel/bpf/verifier.c+2−13

 			       !reg_type_mismatch_ok(prev));
 }
-static bool is_ptr_to_mem_or_btf_id(enum bpf_reg_type type)
-{
-	switch (base_type(type)) {
-	case PTR_TO_MEM:
-	case PTR_TO_BTF_ID:
-		return true;
-	default:
-		return false;
-	}
-}
 static bool is_ptr_to_mem(enum bpf_reg_type type)
 {
 	return base_type(type) == PTR_TO_MEM;
 		 * Reject it.
 		 */
 		if (allow_trust_mismatch &&
-		    is_ptr_to_mem_or_btf_id(type) &&
+		    bpf_is_ptr_to_mem_or_btf_id(type) &&
-		    is_ptr_to_mem_or_btf_id(*prev_type)) {
+		    bpf_is_ptr_to_mem_or_btf_id(*prev_type)) {
 			/*
 			 * Have to support a use case when one path through
 			 * the program yields a TRUSTED pointer while another

bpf: Rewrite any fault prone load out of a mem or btf_id pointer

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code