BPF List
 help / color / mirror / Atom feed
* [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader
@ 2026-05-19 21:53 KP Singh
  2026-05-19 22:50 ` bot+bpf-ci
  2026-05-19 23:17 ` sashiko-bot
  0 siblings, 2 replies; 5+ messages in thread
From: KP Singh @ 2026-05-19 21:53 UTC (permalink / raw)
  To: bpf, linux-security-module; +Cc: ast, daniel, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader
  2026-05-19 21:53 [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader KP Singh
@ 2026-05-19 22:50 ` bot+bpf-ci
  2026-05-19 23:17 ` sashiko-bot
  1 sibling, 0 replies; 5+ messages in thread
From: bot+bpf-ci @ 2026-05-19 22:50 UTC (permalink / raw)
  To: kpsingh, bpf, linux-security-module
  Cc: ast, daniel, kpsingh, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, clm, ihor.solodrai

[-- 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader
  2026-05-19 21:53 [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader KP Singh
  2026-05-19 22:50 ` bot+bpf-ci
@ 2026-05-19 23:17 ` sashiko-bot
  2026-05-20  3:00   ` KP Singh
  1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-05-19 23:17 UTC (permalink / raw)
  To: KP Singh; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The generated BPF loader program uses a hardcoded 8-byte read (BPF_DW) to access the `excl_prog_sha` pointer field, which causes the BPF verifier to reject the loader on 32-bit architectures and silently bypasses the exclusivity check.
- [High] Branch target offset calculation is off-by-one, causing the jump target to land past the beginning of the cleanup block.
--

commit 51df41105aded76b418cec093cbbaaad09c03147
Author: KP Singh <kpsingh@kernel.org>

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

This patch updates the BPF loader to verify that metadata maps are exclusive
while they are frozen. It places excl_prog_sha in struct bpf_map and emits a
check in gen_loader to reject the map with -EINVAL if excl_prog_sha is NULL.

> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> index cd5c2543f54d8..becfda5b90253 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)));

[Severity: High]
Does this 8-byte read (BPF_DW) cause issues on 32-bit architectures?

Since pointers are 4 bytes long on 32-bit systems, the verifier will reject
the loader program with an access size mismatch error (-EACCES) because it
expects the memory access size to exactly match the BTF-defined pointer
size.

Also, if the verifier check were bypassed, wouldn't an 8-byte read straddle
both the 4-byte excl_prog_sha and the adjacent 4-byte ops pointer? Since
ops is always non-zero, the resulting 64-bit value would never be 0,
causing the BPF_JEQ check below to silently fall through and bypass the
exclusivity check.

> +	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));

[Severity: High]
Is the branch target offset calculation off by one?

The offset off is computed based on insn_cur before emitting the
BPF_MOV64_IMM instruction. Since emitting the BPF_MOV64_IMM instruction
advances insn_cur by 8 bytes, and BPF branch offsets are evaluated relative
to the instruction immediately following the branch itself, the actual jump
target evaluates to cleanup_label + 8 bytes (1 instruction) instead of the
cleanup_label itself.

This skips the first instruction of the cleanup block. If nr_progs > 0, it
skips loading the first FD into BPF_REG_1 and calls sys_close() with an
uninitialized or stale register (e.g., a map pointer), potentially closing a
random/incorrect file descriptor in the system. If nr_progs == 0 and
nr_maps > 0, it may jump into the middle of a 16-byte BPF_LD_IMM64
instruction, causing verifier rejection.

Does this need to be adjusted, even though it appears to be a pattern copied
from the loop above it?

> +	} else {
> +		gen->error = -ERANGE;
> +		emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519215358.126364-1-kpsingh@kernel.org?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader
  2026-05-19 23:17 ` sashiko-bot
@ 2026-05-20  3:00   ` KP Singh
  2026-05-21 10:17     ` Alexei Starovoitov
  0 siblings, 1 reply; 5+ messages in thread
From: KP Singh @ 2026-05-20  3:00 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: bpf

On Wed, May 20, 2026 at 1:17 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The generated BPF loader program uses a hardcoded 8-byte read (BPF_DW) to access the `excl_prog_sha` pointer field, which causes the BPF verifier to reject the loader on 32-bit architectures and silently bypasses the exclusivity check.

I plan to move this to the kernel with a kfunc anyways. Any
suggestions for the 32 bit fix (apart from an ugly union in bpf_map).

> - [High] Branch target offset calculation is off-by-one, causing the jump target to land past the beginning of the cleanup block.
> --

Yeah this is valid:

 Compute the cleanup-label offset after emitting the MOV that sets the
  error code in R7. Otherwise the JMP lands one insn past cleanup_label
  and skips the first FD-close, matching the emit_check_err() pattern.

diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index becfda5b9025..6950443ba698 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -592,9 +592,9 @@ static void emit_signature_match(struct bpf_gen *gen)
                gen->hash_insn_offset[i] = gen->insn_cur - gen->insn_start;
                emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_3, 0, 0, 0, 0, 0));

-               off =  -(gen->insn_cur - gen->insn_start -
gen->cleanup_label) / 8 - 1;
+               emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
+               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_REG(BPF_JNE, BPF_REG_2,
BPF_REG_3, off));
                } else {
                        gen->error = -ERANGE;
@@ -610,9 +610,9 @@ static void emit_signature_match(struct bpf_gen *gen)
        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)));
+       emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
        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;

>
> commit 51df41105aded76b418cec093cbbaaad09c03147
> Author: KP Singh <kpsingh@kernel.org>
>
> bpf, libbpf: reject non-exclusive metadata maps in the signed loader
>
> This patch updates the BPF loader to verify that metadata maps are exclusive
> while they are frozen. It places excl_prog_sha in struct bpf_map and emits a
> check in gen_loader to reject the map with -EINVAL if excl_prog_sha is NULL.
>
> > diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> > index cd5c2543f54d8..becfda5b90253 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)));
>
> [Severity: High]
> Does this 8-byte read (BPF_DW) cause issues on 32-bit architectures?
>
> Since pointers are 4 bytes long on 32-bit systems, the verifier will reject
> the loader program with an access size mismatch error (-EACCES) because it
> expects the memory access size to exactly match the BTF-defined pointer
> size.
>
> Also, if the verifier check were bypassed, wouldn't an 8-byte read straddle
> both the 4-byte excl_prog_sha and the adjacent 4-byte ops pointer? Since
> ops is always non-zero, the resulting 64-bit value would never be 0,
> causing the BPF_JEQ check below to silently fall through and bypass the
> exclusivity check.
>
> > +     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));
>
> [Severity: High]
> Is the branch target offset calculation off by one?
>
> The offset off is computed based on insn_cur before emitting the
> BPF_MOV64_IMM instruction. Since emitting the BPF_MOV64_IMM instruction
> advances insn_cur by 8 bytes, and BPF branch offsets are evaluated relative
> to the instruction immediately following the branch itself, the actual jump
> target evaluates to cleanup_label + 8 bytes (1 instruction) instead of the
> cleanup_label itself.
>
> This skips the first instruction of the cleanup block. If nr_progs > 0, it
> skips loading the first FD into BPF_REG_1 and calls sys_close() with an
> uninitialized or stale register (e.g., a map pointer), potentially closing a
> random/incorrect file descriptor in the system. If nr_progs == 0 and
> nr_maps > 0, it may jump into the middle of a 16-byte BPF_LD_IMM64
> instruction, causing verifier rejection.
>
> Does this need to be adjusted, even though it appears to be a pattern copied
> from the loop above it?
>
> > +     } else {
> > +             gen->error = -ERANGE;
> > +             emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
> > +     }
> >  }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260519215358.126364-1-kpsingh@kernel.org?part=1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader
  2026-05-20  3:00   ` KP Singh
@ 2026-05-21 10:17     ` Alexei Starovoitov
  0 siblings, 0 replies; 5+ messages in thread
From: Alexei Starovoitov @ 2026-05-21 10:17 UTC (permalink / raw)
  To: KP Singh, sashiko-reviews; +Cc: bpf

On Tue May 19, 2026 at 8:00 PM PDT, KP Singh wrote:
> On Wed, May 20, 2026 at 1:17 AM <sashiko-bot@kernel.org> wrote:
>>
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>> - [High] The generated BPF loader program uses a hardcoded 8-byte read (BPF_DW) to access the `excl_prog_sha` pointer field, which causes the BPF verifier to reject the loader on 32-bit architectures and silently bypasses the exclusivity check.
>
> I plan to move this to the kernel with a kfunc anyways. Any
> suggestions for the 32 bit fix (apart from an ugly union in bpf_map).

Use _bpf_md_ptr(). It's a normal pattern.

>> - [High] Branch target offset calculation is off-by-one, causing the jump target to land past the beginning of the cleanup block.
>> --
>
> Yeah this is valid:
>
>  Compute the cleanup-label offset after emitting the MOV that sets the
>   error code in R7. Otherwise the JMP lands one insn past cleanup_label
>   and skips the first FD-close, matching the emit_check_err() pattern.
>
> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> index becfda5b9025..6950443ba698 100644
> --- a/tools/lib/bpf/gen_loader.c
> +++ b/tools/lib/bpf/gen_loader.c
> @@ -592,9 +592,9 @@ static void emit_signature_match(struct bpf_gen *gen)
>                 gen->hash_insn_offset[i] = gen->insn_cur - gen->insn_start;
>                 emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_3, 0, 0, 0, 0, 0));
>
> -               off =  -(gen->insn_cur - gen->insn_start -
> gen->cleanup_label) / 8 - 1;
> +               emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
> +               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));

maybe fix the math instead of moving this emit ?
Also split the fix into pre-patch.

>                         emit(gen, BPF_JMP_REG(BPF_JNE, BPF_REG_2,
> BPF_REG_3, off));
>                 } else {
>                         gen->error = -ERANGE;

and while at it remove redundant 'emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));'

pw-bot: cr


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-21 10:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19 21:53 [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader KP Singh
2026-05-19 22:50 ` bot+bpf-ci
2026-05-19 23:17 ` sashiko-bot
2026-05-20  3:00   ` KP Singh
2026-05-21 10:17     ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox