bpf_convert_ctx_accesses() puts a barrier in front of an instruction and may then rewrite the instruction as well. bpf_patch_insn_data() applies the second patch to where the first one left the instruction, the last instruction of the barrier patch. The patch list refuses a second patch for an instruction, since it takes patches in increasing order of the instruction they replace. Make a patch queued again for the same instruction replace the last instruction of the patch queued before. Move the backward jumps of the new instructions down with them, so that a jump back to an earlier instruction, like the one to the shared epilogue, still lands on it. The barrier in front of it doesn't jump, so warn and fail if an instruction in front of the last one jumps past it, instead of moving such jumps along. Signed-off-by: Qiliang Yuan --- kernel/bpf/core.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++-- kernel/bpf/fixups.c | 5 +++-- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 75d6399ce0965..10efd1dd36689 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -599,6 +599,46 @@ static int insn_set_rel(struct bpf_insn *insn, s64 rel, bool is_imm) return 0; } +/* Whether one of the 'len' insns of a patch in front of the last one jumps past it. */ +static bool patch_list_jumps_past(const struct bpf_insn *insns, u32 len) +{ + bool is_imm; + s64 rel; + u32 k; + + for (k = 0; k + 1 < len; k++) + if (insn_get_rel(&insns[k], &rel, &is_imm) && (s64)k + rel + 1 >= len) + return true; + return false; +} + +/* + * The 'len' insns of a patch were moved down by 'by' insns inside it. Keep + * what they referred to in front of the patch, the same way as + * bpf_adj_moved_insn() does: a call to the start of the patch, the old start + * of a subprog, stays there, a jump to itself doesn't. + */ +static int patch_list_move_down(struct bpf_insn *insns, u32 len, u32 by) +{ + bool is_imm, call; + s64 rel, tgt; + u32 k; + int err; + + for (k = 0; k < len; k++) { + if (!insn_get_rel(&insns[k], &rel, &is_imm)) + continue; + call = bpf_pseudo_func(&insns[k]) || bpf_pseudo_call(&insns[k]); + tgt = (s64)k + rel + 1; + if (tgt > 0 || (tgt == 0 && !call)) + continue; + err = insn_set_rel(&insns[k], rel - by, is_imm); + if (err) + return err; + } + return 0; +} + /* * insn is number 'idx' of the 'len' insns that replace insn 'off' of the old * image (an insn that isn't patched is its own patch of length 1). It lands @@ -631,7 +671,10 @@ static int patch_list_adj_insn(struct bpf_insn *insn, const u32 *new_off, * Queue the replacement of insn 'off' of 'prog' by 'patch[0..len)'. Patches * are queued in increasing order of 'off' and applied together, so a pass * keeps addressing insns by their index in 'prog'. Jumps in the patch that - * leave it are relative to the patch placed at 'off'. + * leave it are relative to the patch placed at 'off'. Patching 'off' again + * replaces the last insn of its patch, where the insn being patched went, and + * the insns in front of it must not jump past it. Jumps of the new insns that + * leave the patch are relative to the first of them, as for a first patch. * * Return the copy of the last insn of the patch, which stays valid until the * next call. @@ -640,9 +683,26 @@ struct bpf_insn *__bpf_patch_list_add(struct bpf_patch_list *pl, const struct bpf_prog *prog, u32 off, const struct bpf_insn *patch, u32 len) { - struct bpf_insn_patch *p; + struct bpf_insn_patch *p = pl->cnt ? &pl->patches[pl->cnt - 1] : NULL; int err; + if (p && off == p->off) { + if (WARN_ON_ONCE(patch_list_jumps_past(pl->insns + p->start, p->len))) + return ERR_PTR(-EFAULT); + err = patch_list_reserve_insns(pl, len - 1); + if (err) + return ERR_PTR(err); + /* the last patch is at the end of pl->insns */ + memcpy(pl->insns + p->start + p->len - 1, patch, len * sizeof(*patch)); + err = patch_list_move_down(pl->insns + p->start + p->len - 1, len, + p->len - 1); + if (err) + return ERR_PTR(err); + p->len += len - 1; + pl->insn_cnt += len - 1; + return &pl->insns[p->start + p->len - 1]; + } + err = patch_list_reserve_patch(pl) ?: patch_list_reserve_insns(pl, len); if (err) return ERR_PTR(err); diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index 6c2dce8f3b6eb..a56b743be0062 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -381,16 +381,17 @@ struct bpf_insn *bpf_patch_list_add(struct bpf_verifier_env *env, u32 off, const struct bpf_insn *patch, u32 len) { struct bpf_patch_list *pl = &env->patch_list; + struct bpf_insn_patch *p = pl->cnt ? &pl->patches[pl->cnt - 1] : NULL; if (bpf_rewrite_must_abort()) return ERR_PTR(-EINTR); if (verifier_bug_if(!len || off >= env->prog->len || - (pl->cnt && off <= pl->patches[pl->cnt - 1].off), + (p && (off < p->off || (off == p->off && !p->len))), env, "insn %u patched out of order", off)) return ERR_PTR(-EFAULT); - if (len == 1) { + if (len == 1 && (!p || off != p->off)) { env->prog->insnsi[off] = *patch; adjust_insn_aux_data(env, env->prog, off, 1, NULL, BPF_PATCH_KEEP_TARGET); -- 2.43.0