netdev - Re: bpf jit PPC64 (BE) test_verifier PTR_TO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1552484985.k18yl73ww6.naveen@linux.ibm.com>
Date:   Wed, 13 Mar 2019 19:21:18 +0530
From:   "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Michael Ellerman <mpe@...erman.id.au>,
        Sandipan Das <sandipan@...ux.ibm.com>,
        Yauheni Kaliuta <yauheni.kaliuta@...hat.com>
Cc:     bpf@...r.kernel.org, Jiri Olsa <jolsa@...hat.com>,
        linuxppc-dev@...ts.ozlabs.org, netdev@...r.kernel.org
Subject: Re: bpf jit PPC64 (BE) test_verifier PTR_TO_STACK store/load failure

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