Re: [bpf-next 1/4] selftests/bpf: map_kptr: force BPF_STX for the scalar store to kptr

On 8/5/26 10:47 AM, Yonghong Song wrote:
>
> On 8/3/26 10:02 AM, Vineet Gupta wrote:
>> reject_scalar_store_to_kptr stores a scalar constant to a kptr field:
>>
>>           *(volatile u64 *)&v->unref_ptr = 0xBADC0DE;
>>
>> Compilers generate one of two encodings for that:
>>
>>    1. Materialize the constant into a register and emit BPF_STX:
>>
>>           r1 = 0xbadc0de
>>           *(u64 *)(r0 + 0x8) = r1
>>
>>    2. Or fold it into a single BPF_ST (store immediate):
>>
>>           *(u64 *)(r0 + 0x8) = 0xbadc0de
>>
>> check_map_kptr_access() rejects both, but through very different checks.
>> BPF_STX goes through map_kptr_match_type(), whose first test is
>> base_type(reg->type) != PTR_TO_BTF_ID - the scalar rejection this test is
>> named for - and which prints "invalid kptr access, R...". BPF_ST only gets
>> the trivial "BPF_ST imm must be 0 when storing to kptr" immediate check and
>> never reaches map_kptr_match_type() at all.
>>
>> So on a compiler that folds the constant - bpf-gcc, and clang from
>> -mcpu=v4, which enabled BPF_ST around v4 support due to historical
>> verifier limitations - the test fails against its expected message.
>>
>> Widening the __msg to accept either message would make it pass again, but
>> on those toolchains it would then only re-test the imm != 0 path, which
>> verifier/map_kptr.c ("map_kptr: BPF_ST imm != 0") already covers, and the
>> scalar-vs-PTR_TO_BTF_ID check would lose its only test in the tree.
>>
>> Route the value through barrier_var() instead, so the store stays a
>> BPF_STX everywhere and the test keeps asserting what it was written to
>> assert. clang -mcpu=v1..v4 and bpf-gcc 16.1 all emit the register form
>> afterwards.
>>
>>     bpf-gcc, before: #229/20 map_kptr/reject_scalar_store_to_kptr:FAIL
>>     bpf-gcc, after : #229/20 map_kptr/reject_scalar_store_to_kptr:OK
>>
>> Signed-off-by: Vineet Gupta <[email protected]>
>> ---
>>    tools/testing/selftests/bpf/progs/map_kptr_fail.c | 11 ++++++++++-
>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
>> index f11848dfa78f..cb84e23b83c0 100644
>> --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
>> +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
>> @@ -390,13 +390,22 @@ __failure __msg("invalid kptr access, R")
>>    int reject_scalar_store_to_kptr(struct __sk_buff *ctx)
>>    {
>>    	struct map_value *v;
>> +	u64 val = 0xBADC0DE;
>>    	int key = 0;
>>    
>>    	v = bpf_map_lookup_elem(&array_map, &key);
>>    	if (!v)
>>    		return 0;
>>    
>> -	*(volatile u64 *)&v->unref_ptr = 0xBADC0DE;
>> +	/*
>> +	 * Keep the value in a register so this stays a BPF_STX and keeps
>> +	 * exercising map_kptr_match_type(). Compilers that fold the constant
>> +	 * into a BPF_ST (store immediate) instead - bpf-gcc, and clang from
>> +	 * -mcpu=v4 - would be rejected by the far weaker "BPF_ST imm must be
>> +	 * 0" check, which verifier/map_kptr.c already covers.
>> +	 */
>> +	barrier_var(val);
>> +	*(volatile u64 *)&v->unref_ptr = val;
> Could you explain in details why gcc16 (with cpuv4) won't work with BPF_ST?
> What code gcc16 (with cpuv4) generates?

GCC generates BPF_ST (and clang -mcpu=v4 does as well)

    7: (7a) *(u64 *)(r0 +8) = 195936478
    BPF_ST imm must be 0 when storing to kptr at off=8
    processed 7 insns (limit 1000000) max_states_per_insn 0 total_states
    0 peak_states 0 mark_read 0
    =============
    EXPECTED   SUBSTR: 'invalid kptr access, R'


The issue is not codegen or BPF_ST vs. BPF_STX, its a deliberate bad 
write to a pointer.
The current __msg in test only matches v3 (STX) form. v4's 
store-immediate hits a different verifier check with a different message.
My first approach was to support both in the __msg, but then Claude 
suggested to not do it that way because it would reduce coverage as 
mentioned in the changelog above.

Thx
-Vineet