Thanks! On Mon, 1 Mar 2021 at 16:40, Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote: > > insn_has_def32() returns false for 32-bit BPF_FETCH insns. This makes > adjust_insn_aux_data() incorrectly set zext_dst, as can be seen in [1]. > This happens because insn_no_def() does not know about the BPF_FETCH > variants of BPF_STX. > > Fix in two steps. > > First, replace insn_no_def() with insn_def_regno(), which returns the > register an insn defines. Normally insn_no_def() calls are followed by > insn->dst_reg uses; replace those with the insn_def_regno() return > value. > > Second, adjust the BPF_STX special case in is_reg64() to deal with > queries made from opt_subreg_zext_lo32_rnd_hi32(), where the state > information is no longer available. Add a comment, since the purpose > of this special case is not clear at first glance. > > [1] https://lore.kernel.org/bpf/20210223150845.1857620-1-jackmanb@xxxxxxxxxx/ > > Fixes: 5ffa25502b5a ("bpf: Add instructions for atomic_[cmp]xchg") > Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx> > Acked-by: Martin KaFai Lau <kafai@xxxxxx> > --- > > v1: https://lore.kernel.org/bpf/20210224141837.104654-1-iii@xxxxxxxxxxxxx/ > v1 -> v2: Per Martin's comments: rebase against the bpf branch, fix the > Fixes: tag, fix the comment style, replace ?: with the more > readable if-else, handle the internal verifier error using > WARN_ON_ONCE(), verbose() and -EFAULT. > > v2: https://lore.kernel.org/bpf/20210226213131.118173-1-iii@xxxxxxxxxxxxx/ > v2 -> v3: Per Brendan's comment, add "verifier bug." to the error > message. Unfortunately, the load_reg assignment cannot be > moved, because this would also require moving the insn > assignment, and this would ruin the reverse xmas tree. Acked-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
