Re: bpf jit PPC64 (BE) test_verifier PTR_TO_STACK store/load failure

["Naveen N. Rao" <naveen.n.rao@linux.ibm.com>]

Hi,

Yauheni Kaliuta wrote:
> Hi!
> 
> I found a failure:
> 
> ```
> # ./test_verifier 722
> #722/u PTR_TO_STACK store/load FAIL retval -1 != -87117812 
> 0: (bf) r1 = r10
> 1: (07) r1 += -10
> 2: (7a) *(u64 *)(r1 +2) = -87117812
> 3: (79) r0 = *(u64 *)(r1 +2)
> 4: (95) exit
> processed 5 insns (limit 131072), stack depth 8
> #722/p PTR_TO_STACK store/load FAIL retval -1 != -87117812 
> 0: (bf) r1 = r10
> 1: (07) r1 += -10
> 2: (7a) *(u64 *)(r1 +2) = -87117812
> 3: (79) r0 = *(u64 *)(r1 +2)
> 4: (95) exit
> processed 5 insns (limit 131072), stack depth 8
> Summary: 0 PASSED, 0 SKIPPED, 2 FAILED
> ```
> 
> The reason is in the JIT. The code is jitted into:
> 
> [...]
> d00000000580e7f8:       f9 23 00 00     std     r9,0(r3)
> d00000000580e7fc:       e9 03 00 02     lwa     r8,0(r3)
> [...]
> 
> so, it stores DW to the location r3, but loads W, i.e. in BE it is:
> 
> saves
> r3: FF FF FF FF FA CE B0 0C
> loads
> r3: FF FF FF FF
> 
> (in LE it works semicorretly, saves 0C B0 CE FA FF FF FF FF, loads 0C B0 CE FA)
> 
> This is because of the handling of the +2 offset. For stores it is:
> 
> 
> #define PPC_STD(r, base, i)	EMIT(PPC_INST_STD | ___PPC_RS(r) |	      
> \
> 				     ___PPC_RA(base) | ((i) & 0xfffc))
> 
> and for loads
> #define PPC_LD(r, base, i)	EMIT(PPC_INST_LD | ___PPC_RT(r) |	      \
> 				     ___PPC_RA(base) | IMM_L(i))
> #define IMM_L(i)		((uintptr_t)(i) & 0xffff)
> 
> So, in the load case the offset +2 (immediate value) is not
> masked and turns the instruction to lwa instead of ld.

Indeed -- good catch and analysis!

> 
> 
> Would it be correct to & 0xfffc the immediate value as well?

Yes, I think that would be the right fix.

> 
> BTW, the full run on big endian:
> 
> Summary: 1190 PASSED, 125 SKIPPED, 4 FAILED

Thanks for pointing that out, I'll look into these failures.


- Naveen