From 290e0052fc7f47b49468f17a20dd6ba570ebbe1d Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Wed, 9 Sep 2020 22:46:44 -0700 Subject: [PATCH] If the original insn is a jump, then it is not subjected to branch adjustment, which is incorrect. As discovered by Yauheni in https://lore.kernel.org/bpf/20200903140542.156624-1-yauheni.kaliuta@redhat.com/ this causes `test_progs -t global_funcs` failures on s390. Most likely, the current code includes the original insn in the patchlet, because there was no infrastructure to insert new insns, only to replace the existing ones. Now that bpf_patch_insns_data() can do insertions, stop including the original insns in zext patchlets. Fixes: a4b1d3c1ddf6 ("bpf: verifier: insert zero extension according to analysis result") Reported-by: Yauheni Kaliuta Signed-off-by: Ilya Leoshkevich --- kernel/bpf/verifier.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) --- kernel/bpf/verifier.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 17c2e926e4369..64a04953c631c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9911,7 +9911,7 @@ static int opt_remove_nops(struct bpf_verifier_env *env) static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, const union bpf_attr *attr) { - struct bpf_insn *patch, zext_patch[2], rnd_hi32_patch[4]; + struct bpf_insn *patch, zext_patch, rnd_hi32_patch[4]; struct bpf_insn_aux_data *aux = env->insn_aux_data; int i, patch_len, delta = 0, len = env->prog->len; struct bpf_insn *insns = env->prog->insnsi; @@ -9919,13 +9919,14 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, bool rnd_hi32; rnd_hi32 = attr->prog_flags & BPF_F_TEST_RND_HI32; - zext_patch[1] = BPF_ZEXT_REG(0); + zext_patch = BPF_ZEXT_REG(0); rnd_hi32_patch[1] = BPF_ALU64_IMM(BPF_MOV, BPF_REG_AX, 0); rnd_hi32_patch[2] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_AX, 32); rnd_hi32_patch[3] = BPF_ALU64_REG(BPF_OR, 0, BPF_REG_AX); for (i = 0; i < len; i++) { int adj_idx = i + delta; struct bpf_insn insn; + int len_old = 1; insn = insns[adj_idx]; if (!aux[adj_idx].zext_dst) { @@ -9968,20 +9969,21 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, if (!bpf_jit_needs_zext()) continue; - zext_patch[0] = insn; - zext_patch[1].dst_reg = insn.dst_reg; - zext_patch[1].src_reg = insn.dst_reg; - patch = zext_patch; - patch_len = 2; + zext_patch.dst_reg = insn.dst_reg; + zext_patch.src_reg = insn.dst_reg; + patch = &zext_patch; + patch_len = 1; + adj_idx++; + len_old = 0; apply_patch_buffer: - new_prog = bpf_patch_insns_data(env, adj_idx, 1, patch, + new_prog = bpf_patch_insns_data(env, adj_idx, len_old, patch, patch_len); if (!new_prog) return -ENOMEM; env->prog = new_prog; insns = new_prog->insnsi; aux = env->insn_aux_data; - delta += patch_len - 1; + delta += patch_len - len_old; } return 0;