[LTP] [PATCH v2 4/4] bpf_prog05: Drop CAP_BPF and check if ptr arithmetic is allowed

[Joerg Vehlow]

Hi Richard,

Am 1/11/2022 um 3:36 PM schrieb Richard Palethorpe:
>>> +	if (log[0] != 0)
>>> +		tst_brk(TCONF | TERRNO, "No pointer arithmetic:\n %s", log);
>> This check now fails for me with the following output, where the test
>> was successful, before this patch. The kernel is a non-standard suse
>> 5.3.18 with realtime patches.
>>
>> bpf_prog05.c:100: TCONF: No pointer arithmetic:
>>   0: (bf) r2 = r10
>> 1: (b7) r3 = -1
>> 2: (0f) r2 += r3
>> 3: (72) *(u8 *)(r2 +0) = 0
>> 4: (b7) r0 = 0
>> 5: (95) exit
>>
>> from 2 to 3 (speculative execution): R1=ctx(id=0,off=0,imm=0) R2_w=fp0
>> R3_w=invP-1 R10=fp0
>> 3: (72) *(u8 *)(r2 +0) = 0
>> invalid stack off=0 size=1
>> processed 7 insns (limit 1000000) max_states_per_insn 0 total_states 0
>> peak_states 0 mark_read 0
>> : EACCES (13)
>>
>>
>> Is this too old? But then again, the test was successful before this
>> patch and your commit message states, that it was not successful on
>> "older kernels".
> 
> Are you testing on ARM? eBPF adopted the ARM behavior when dividing by
> zero.
> 

No this is x86 and I was expecting 5.3 to not be "very old". IIRC 
patches for the bug tested here are partly already integrated into 4.19, 
so I guess 5.3 shouldn't be "very old" in that context?

I am not sure what you tried to test, I guess accessing the stack using 
indirect pointer arithmetic? Because changing the store to "*(u8 *)(r10 
- 1) = 0", works. But if this indirection is required for the actually 
test, it should fail, but it doesn't -> I think the check tests 
something, that is not actually required for the actual test.
I guess what you wanted to check is what BPF_MAP_ARRAY_STX does. There 
is one major difference between your implementation of the check and 
BPF_MAP_ARRAY_STX: BPF_MAP_ARRAY_STX uses an immediate value, to add to 
r2 compared to a register in your check:

  * r2 = fp
  * r2 += r2 - 4

vs

  * r2 = r10
  * r3 = -1
  * r2 += r3

That is actually what makes the difference here...

If I modify the check like this it works:

--- a/testcases/kernel/syscalls/bpf/bpf_prog05.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog05.c
@@ -67,13 +67,11 @@ static void ensure_ptr_arithmetic(void)
  {
         const struct bpf_insn prog_insn[] = {
                 /* r2 = r10
-                * r3 = -1
-                * r2 += r3
+                * r2 += -1
                  * *(char *)r2 = 0
                  */
                 BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
-               BPF_MOV64_IMM(BPF_REG_3, -1),
-               BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_3),
+               BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -1),
                 BPF_ST_MEM(BPF_B, BPF_REG_2, 0, 0),

                 /* exit(0) */


Is this still checking what it is supposed to? Then I would send it post 
this as a patch

Joerg