[PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader

[PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader

[KP Singh]

The loader verifies map->sha against the metadata hash in its
instructions. map->sha is calculated when BPF_OBJ_GET_INFO_BY_FD is called
on the frozen map.

While the map is frozen, the loader must also ensure the map is
exclusive, as, without exclusivity, another BPF program with map access
can mutate the contents afterwards, so the check passes on stale data.

Place excl_prog_sha right after sha[] in struct bpf_map and have
gen_loader bail with -EINVAL when it is NULL, via BPF_PSEUDO_MAP_IDX at
fixed offset 32.

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/bpf.h                             |  2 +-
 tools/lib/bpf/gen_loader.c                      | 17 +++++++++++++++++
 .../selftests/bpf/progs/verifier_map_ptr.c      |  2 +-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index cd191c5fdb0a..11bec73db199 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -295,6 +295,7 @@ struct bpf_map_owner {
 
 struct bpf_map {
 	u8 sha[SHA256_DIGEST_SIZE];
+	char *excl_prog_sha;
 	const struct bpf_map_ops *ops;
 	struct bpf_map *inner_map_meta;
 #ifdef CONFIG_SECURITY
@@ -335,7 +336,6 @@ struct bpf_map {
 	atomic64_t sleepable_refcnt;
 	s64 __percpu *elem_count;
 	u64 cookie; /* write-once */
-	char *excl_prog_sha;
 };
 
 static inline const char *btf_field_type_name(enum btf_field_type type)
diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index cd5c2543f54d..becfda5b9025 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -601,6 +601,23 @@ static void emit_signature_match(struct bpf_gen *gen)
 			emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
 		}
 	}
+
+	/* Reject if the metadata map is not exclusive. Without exclusivity
+	 * the cached map->sha[] verified above can be stale: another BPF
+	 * program with map access could have mutated the contents between
+	 * BPF_OBJ_GET_INFO_BY_FD and loader execution.
+	 */
+	emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX,
+					 0, 0, 0, 0));
+	emit(gen, BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1, SHA256_DWORD_SIZE * sizeof(__u64)));
+	off = -(gen->insn_cur - gen->insn_start - gen->cleanup_label) / 8 - 1;
+	if (is_simm16(off)) {
+		emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
+		emit(gen, BPF_JMP_IMM(BPF_JEQ, BPF_REG_2, 0, off));
+	} else {
+		gen->error = -ERANGE;
+		emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
+	}
 }
 
 void bpf_gen__record_attach_target(struct bpf_gen *gen, const char *attach_name,
diff --git a/tools/testing/selftests/bpf/progs/verifier_map_ptr.c b/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
index e2767d27d8aa..f59e8d8d3b18 100644
--- a/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
+++ b/tools/testing/selftests/bpf/progs/verifier_map_ptr.c
@@ -76,7 +76,7 @@ __naked void bpf_map_ptr_write_rejected(void)
 SEC("socket")
 __description("bpf_map_ptr: read non-existent field rejected")
 __failure
-__msg("cannot access ptr member ops with moff 32 in struct bpf_map with off 33 size 4")
+__msg("cannot access ptr member excl_prog_sha with moff 32 in struct bpf_map with off 33 size 4")
 __failure_unpriv
 __msg_unpriv("access is allowed only to CAP_PERFMON and CAP_SYS_ADMIN")
 __flag(BPF_F_ANY_ALIGNMENT)
-- 
2.53.0

Re: [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]

> Signed-off-by: KP Singh <kpsingh@kernel.org>

This looks like a bug fix for the signed loader.  Should this include:

  Fixes: 66a1025f7f0b ("Merge tag 'soc-newsoc-6.19' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc")

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h

[ ... ]

> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> index cd5c2543f54d..becfda5b9025 100644
> --- a/tools/lib/bpf/gen_loader.c
> +++ b/tools/lib/bpf/gen_loader.c
> @@ -601,6 +601,23 @@ static void emit_signature_match(struct bpf_gen *gen)
>  			emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
>  		}
>  	}
> +
> +	/* Reject if the metadata map is not exclusive. Without exclusivity
> +	 * the cached map->sha[] verified above can be stale: another BPF
> +	 * program with map access could have mutated the contents between
> +	 * BPF_OBJ_GET_INFO_BY_FD and loader execution.
> +	 */

This isn't a bug, but the BPF subsystem coding-style rule requires
multi-line comments to have the opening /* on its own line, with the
comment text starting on the next line. This applies to tools/lib/bpf/
even when surrounding code in the same file uses the old style:

    /*
     * Reject if the metadata map is not exclusive. Without exclusivity
     * the cached map->sha[] verified above can be stale: another BPF
     * program with map access could have mutated the contents between
     * BPF_OBJ_GET_INFO_BY_FD and loader execution.
     */

> +	emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX,
> +					 0, 0, 0, 0));
> +	emit(gen, BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1, SHA256_DWORD_SIZE * sizeof(__u64)));
> +	off = -(gen->insn_cur - gen->insn_start - gen->cleanup_label) / 8 - 1;
> +	if (is_simm16(off)) {
> +		emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
> +		emit(gen, BPF_JMP_IMM(BPF_JEQ, BPF_REG_2, 0, off));
> +	} else {
> +		gen->error = -ERANGE;
> +		emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
> +	}
>  }

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26127913079