[PATCH bpf-next v5 01/16] bpf: Check exclusive maps once the program digest is final

[Kumar Kartikeya Dwivedi <memxor@gmail.com>]

An exclusive map is bound to the digest of its program, which the
verifier compared against prog->digest in check_map_prog_compatibility()
as the map was added to the program. For a map an insn refers to that
happens in check_and_resolve_insns(), after the digest is computed at
the top of that function. For a map bound through a continuous fd_array
it happens in process_fd_array(), which runs earlier, so for an unsigned
program the comparison saw a zeroed digest and the load failed with
-EACCES, "program's hash doesn't match map's excl_prog_hash", for the
very program the map is exclusive to.

Computing the digest earlier would not do: it has to cover the insns as
the kernel verifies and runs them, after the CO-RE relocations it
applies, or a loader could submit the insns of the exclusive program
together with relocations that change them and pass the check with a
different program. Leave the digest where it is and move the comparison
instead: check every used map against the digest at the end of
check_and_resolve_insns(), whether an insn referred to it or the array
bound it.

The claims a program makes on its maps move behind the check as well:
bpf_map_claim(), which records the program as the user of the map for
resolve_func_ptrs(), and the takeover of an insn array by
bpf_insn_array_init(). Both ran as the map was added, so a program that
is not the owner of an exclusive map could have flipped the owner's
single-user state to shared, which a failed load does not undo, or held
the owner's insn array while it was being verified. Now they run once
every map has passed the check, and a rejected program leaves the maps
as it found them. Nothing consults a claim before resolve_func_ptrs()
and the fixup passes, which run later.

The early digest of a signed program goes with the early comparison.
Commit b707068e0ed9 ("bpf: Verify signed loader metadata at load time")
computed it only so that the maps of a signed loader passed the check in
process_fd_array(); the signature itself is verified over the submitted
insns and the frozen maps by bpf_prog_verify_signature(), which does not
read the digest. The maps of a signed program are now checked against
the digest of the final insns like everyone else's, which covers the
relocations the kernel applies and the signature does not; a loader
program carries none.

Fixes: baefdbdf6812 ("bpf: Implement exclusive map creation")
Cc: KP Singh <kpsingh@kernel.org>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 85 ++++++++++++++++++++++++++++++-------------
 1 file changed, 59 insertions(+), 26 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d13868846823..ca5301fe3f66 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -20796,12 +20796,6 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
 {
 	enum bpf_prog_type prog_type = resolve_prog_type(prog);
 
-	if (map->excl_prog_sha &&
-	    memcmp(map->excl_prog_sha, prog->digest, SHA256_DIGEST_SIZE)) {
-		verbose(env, "program's hash doesn't match map's excl_prog_hash\n");
-		return -EACCES;
-	}
-
 	if (btf_record_has_field(map->record, BPF_LIST_HEAD) ||
 	    btf_record_has_field(map->record, BPF_RB_ROOT)) {
 		if (is_tracing_prog_type(prog_type)) {
@@ -20899,6 +20893,61 @@ static int bpf_map_claim(struct bpf_verifier_env *env, struct bpf_map *map)
 	}
 }
 
+/*
+ * An exclusive map is bound to the digest of its program. The digest covers
+ * the insns as they are verified, after the CO-RE relocations the kernel
+ * applies, so it exists only once check_and_resolve_insns() has computed it,
+ * and the maps are checked against it there, whether an insn referred to
+ * them or a continuous fd_array bound them before.
+ */
+static int check_excl_maps(struct bpf_verifier_env *env)
+{
+	int i;
+
+	for (i = 0; i < env->used_map_cnt; i++) {
+		struct bpf_map *map = env->used_maps[i];
+
+		if (map->excl_prog_sha &&
+		    memcmp(map->excl_prog_sha, env->prog->digest, SHA256_DIGEST_SIZE)) {
+			verbose(env, "program's hash doesn't match map's excl_prog_hash\n");
+			return -EACCES;
+		}
+	}
+	return 0;
+}
+
+/*
+ * The program may use every map it referred to, see check_excl_maps(). Only now
+ * does it leave a trace on them: it claims each as its user, see
+ * bpf_map_claim(), and takes over its insn arrays. A program rejected before
+ * this point, one that is not the owner of an exclusive map in particular,
+ * leaves the maps as it found them. Nothing consults a claim before
+ * resolve_func_ptrs() and the fixup passes, which run later.
+ */
+static int claim_used_maps(struct bpf_verifier_env *env)
+{
+	int i, err;
+
+	for (i = 0; i < env->used_map_cnt; i++) {
+		struct bpf_map *map = env->used_maps[i];
+
+		err = bpf_map_claim(env, map);
+		if (err)
+			return err;
+
+		if (map->map_type != BPF_MAP_TYPE_INSN_ARRAY)
+			continue;
+		err = bpf_insn_array_init(map, env->prog);
+		if (err) {
+			verbose(env, "Failed to properly initialize insn array\n");
+			return err;
+		}
+		env->insn_array_maps[env->insn_array_map_cnt++] = map;
+		env->prog->jit_required = true;
+	}
+	return 0;
+}
+
 static int __add_used_map(struct bpf_verifier_env *env, struct bpf_map *map)
 {
 	int i, err;
@@ -20936,20 +20985,6 @@ static int __add_used_map(struct bpf_verifier_env *env, struct bpf_map *map)
 
 	env->used_maps[env->used_map_cnt++] = map;
 
-	err = bpf_map_claim(env, map);
-	if (err)
-		return err;
-
-	if (map->map_type == BPF_MAP_TYPE_INSN_ARRAY) {
-		err = bpf_insn_array_init(map, env->prog);
-		if (err) {
-			verbose(env, "Failed to properly initialize insn array\n");
-			return err;
-		}
-		env->insn_array_maps[env->insn_array_map_cnt++] = map;
-		env->prog->jit_required = true;
-	}
-
 	return env->used_map_cnt - 1;
 }
 
@@ -21353,7 +21388,10 @@ static int check_and_resolve_insns(struct bpf_verifier_env *env)
 	 * 'struct bpf_map *' into a register instead of user map_fd.
 	 * These pointers will be used later by verifier to validate map access.
 	 */
-	return 0;
+	err = check_excl_maps(env);
+	if (err)
+		return err;
+	return claim_used_maps(env);
 }
 
 /* that many pointers to functions are recognized in the maps of a program at most */
@@ -23619,11 +23657,6 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
 		ret = -EINVAL;
 		goto err_prep;
 	}
-	if (env->signature) {
-		ret = bpf_prog_calc_tag(env->prog);
-		if (ret < 0)
-			goto err_prep;
-	}
 
 	ret = process_fd_array(env, attr, uattr);
 	if (ret)
-- 
2.53.0-Meta