Re: [PATCH bpf-next v3 2/3] selftests/bpf: add cmp_map_pointer_with_const test

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 6/5/25 9:08 AM, Alexei Starovoitov wrote:
> On Wed, Jun 4, 2025 at 8:04 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> On 6/4/25 3:41 PM, Alexei Starovoitov wrote:
>>> On Wed, Jun 4, 2025 at 3:28 PM Ihor Solodrai <isolodrai@meta.com> wrote:
>>>>
>>>> Add a test for CONST_PTR_TO_MAP comparison with a non-0 constant. A
>>>> BPF program with this code must not pass verification in unpriv.
>>>>
>>>> Signed-off-by: Ihor Solodrai <isolodrai@meta.com>
>>>> ---
>>>>    .../selftests/bpf/progs/verifier_unpriv.c       | 17 +++++++++++++++++
>>>>    1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/progs/verifier_unpriv.c b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
>>>> index 28200f068ce5..c4a48b57e167 100644
>>>> --- a/tools/testing/selftests/bpf/progs/verifier_unpriv.c
>>>> +++ b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
>>>> @@ -634,6 +634,23 @@ l0_%=:     r0 = 0;                                         \
>>>>           : __clobber_all);
>>>>    }
>>>>
>>>> +SEC("socket")
>>>> +__description("unpriv: cmp map pointer with const")
>>>> +__success __failure_unpriv __msg_unpriv("R1 pointer comparison prohibited")
>>>> +__retval(0)
>>>> +__naked void cmp_map_pointer_with_const(void)
>>>> +{
>>>> +       asm volatile ("                                 \
>>>> +       r1 = 0;                                         \
>>>> +       r1 = %[map_hash_8b] ll;                         \
>>>> +       if r1 == 0xdeadbeef goto l0_%=;         \
>>>
>>> I bet this doesn't fit into imm32 either.
>>> It should fit into _signed_ imm32.
>>
>> Apparently it's fine both for gcc and clang:
>> https://github.com/kernel-patches/bpf/actions/runs/15454151804
> 
> Both compilers are buggy then.
> 
>> I guess the value from inline asm is just put into IMM bytes as
>> is. llvm-objdump is exactly the same, although the value is pretty
>> printed as negative:
>>
>> 0000000000000320 <cmp_map_pointer_with_const>:
>>        100:       b7 01 00 00 00 00 00 00 r1 = 0x0
>>        101:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x0 ll
>>        103:       15 01 00 00 ef be ad de if r1 == -0x21524111 goto +0x0
> 
> It's 64-bit 0xFFFFffffdeadbeef
> Not the same as 0xdeadbeef

I am not sure what the issue is, would appreciate an explanation.

Inline asm contains a 32bit literal (without a sign). Compiler takes
this literal as is and puts it into imm field of the instruction,
which is also 32bit. The instruction is valid and this value _means_
signed integer, in particular for the verifier.

Are you saying that compiler should check the sign of the literal and
verify it's in signed 32bit range? In other words if you want
0xdeadbeef bytes in the imm, you must write -0x21524111 in the asm?

AFAIU it'd be different from C then, because you can write:

    int k = 0xdeadbeef;
    printf("%d\n", k); // prints -559038737

and it's fine.

Looking at Yonghong's llvm pr [1], it will not error for 0xdeadbeef
because it's less than UINT_MAX:

     if (MO.isImm()) {
         int64_t Imm = MO.getImm();
         if (MI.getOpcode() != BPF::LD_imm64 && (Imm < INT_MIN || Imm > 
UINT_MAX))
           Ctx.reportError(MI.getLoc(),
                           "immediate out of range, shall fit in 32 bits");
         return static_cast<unsigned>(Imm);
       }

[1] https://github.com/llvm/llvm-project/pull/142989