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

[Daniel Borkmann <daniel@iogearbox.net>]

On 5/22/26 11:44 PM, Daniel Borkmann wrote:
> On 5/21/26 5:22 PM, KP Singh wrote:
>> 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. Declare excl_prog_sha with __bpf_md_ptr so the
>> 8-byte BPF_LDX_MEM read works on 32-bit kernels.
>>
>> Fixes: fb2b0e290147 ("libbpf: Update light skeleton for signing")
>> Signed-off-by: KP Singh <kpsingh@kernel.org>
>> ---
>>   include/linux/bpf.h                              |  2 +-
>>   tools/lib/bpf/gen_loader.c                       | 16 ++++++++++++++++
>>   .../selftests/bpf/progs/verifier_map_ptr.c       | 10 ++++++----
>>   3 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index cd191c5fdb0a..ea9bd24f82c0 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];
>> +    __bpf_md_ptr(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 9478b8f78f26..fee35c26deb8 100644
>> --- a/tools/lib/bpf/gen_loader.c
>> +++ b/tools/lib/bpf/gen_loader.c
>> @@ -600,6 +600,22 @@ static void emit_signature_match(struct bpf_gen *gen)
>>               gen->error = -ERANGE;
>>           }
>>       }
>> +
>> +    /* 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.
>> +     */

(One other small nit: it probably makes sense to first check the exclusivity
before we compare the map->sha[], so this could be reordered inside the
emit_signature_match.)

> We could probably add sth like this somewhere in the bpf kernel side :
> 
>          /* See: emit_signature_match() in tools/lib/bpf/gen_loader.c */
>          BUILD_BUG_ON(offsetof(struct bpf_map, sha) != 0);
>          BUILD_BUG_ON(offsetof(struct bpf_map, excl_prog_sha) != SHA256_DIGEST_SIZE);
>          BUILD_BUG_ON(sizeof_field(struct bpf_map, excl_prog_sha) != sizeof(__u64));
> 
>> +    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)));
> 
> nit: SHA256_DWORD_SIZE is SHA256_DIGEST_LENGTH / sizeof(__u64), can we just use
>       SHA256_DIGEST_LENGTH ?
> 
>> +    off = -(gen->insn_cur - gen->insn_start - gen->cleanup_label) / 8 - 2;
>> +    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;
>> +    }
>>   }
>>   void bpf_gen__record_attach_target(struct bpf_gen *gen, const char *attach_name,
>