Messages in this thread | | | Date | Mon, 26 Aug 2019 12:29:05 +0530 | From | "Naveen N. Rao" <> | Subject | Re: [PATCH] bpf: handle 32-bit zext during constant blinding |
| |
Jiong Wang wrote: > > Naveen N. Rao writes: > >> Since BPF constant blinding is performed after the verifier pass, the >> ALU32 instructions inserted for doubleword immediate loads don't have a >> corresponding zext instruction. This is causing a kernel oops on powerpc >> and can be reproduced by running 'test_cgroup_storage' with >> bpf_jit_harden=2. >> >> Fix this by emitting BPF_ZEXT during constant blinding if >> prog->aux->verifier_zext is set. >> >> Fixes: a4b1d3c1ddf6cb ("bpf: verifier: insert zero extension according to analysis result") >> Reported-by: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > > Thanks for the fix. > > Reviewed-by: Jiong Wang <jiong.wang@netronome.com> > > Just two other comments during review in case I am wrong on somewhere. > > - Use verifier_zext instead of bpf_jit_needs_zext() seems better, even > though the latter could avoid extending function argument. > > Because JIT back-ends look at verifier_zext, true means zext inserted > by verifier so JITs won't do the code-gen. > > Use verifier_zext is sort of keeping JIT blinding the same behaviour > has verifier even though blinding doesn't belong to verifier, but for > such insn patching, it could be seen as a extension of verifier, > therefore use verifier_zext seems better than bpf_jit_needs_zext() to > me. > > - JIT blinding is also escaping the HI32 randomization which happens > inside verifier, otherwise x86-64 regression should have caught this issue.
Jiong, Thanks for the review.
Alexei, Daniel, Can you please pick this up for v5.3. This is a regression and is causing a crash on powerpc.
- Naveen
| |