> bpf: Add a list of deferred instruction patches
TL;DR: Nit only: patch_list_adj_aux() duplicates the aux-data propagation
logic of adjust_insn_aux_data() in fixups.c; the review asks whether a shared
helper could keep those rules in one place. Not a bug.
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 37cf130ebb57..39566f382510 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -404,6 +404,301 @@ struct bpf_prog *bpf_patch_insn_data(struct
> bpf_verifier_env *env, u32 off,
[ ... ]
> +/*
> + * Fill the aux data of the 'p->len' insns at 'slot' from the aux data 'old'
> + * of the insn they replace, the same way as adjust_insn_aux_data() does.
> + */
> +static void patch_list_adj_aux(struct bpf_prog *prog, struct bpf_insn_patch
> *p,
> + struct bpf_insn *patch,
> + const struct bpf_insn_aux_data *old,
> + struct bpf_insn_aux_data *slot)
> +{
> + struct bpf_insn_aux_data *last = slot + p->len - 1;
> + struct bpf_insn *orig = &p->orig;
> + u32 i;
> +
> + *last = *old;
> + last->zext_dst = bpf_insn_def32(prog, &patch[p->len - 1]) >= 0;
> + memset(slot, 0, sizeof(*slot) * (p->len - 1));
> +
> + for (i = 0; i < p->len - 1; i++) {
> + slot[i].seen = last->seen;
> + slot[i].zext_dst = bpf_insn_def32(prog, &patch[i]) >= 0;
> + if (!memcmp(&patch[i], orig, sizeof(*orig))) {
> + slot[i].non_stack_access = last->non_stack_access;
> + last->non_stack_access = false;
> + } else if (bpf_is_mem_insn(&patch[i])) {
> + slot[i].non_stack_access = true;
> + }
> + }
> +
> + if (bpf_is_mem_insn(&patch[p->len - 1]) &&
> + memcmp(&patch[p->len - 1], orig, sizeof(*orig)))
> + last->non_stack_access = true;
> +
> + /* indirect jumps to the replaced insn land on the first new one */
> + if (last->indirect_target) {
> + slot[0].indirect_target = 1;
> + last->indirect_target = 0;
> + }
> +}
this isn't a bug, but patch_list_adj_aux() repeats the per-slot aux fill
that adjust_insn_aux_data() already does for the BPF_PATCH_KEEP_TARGET
case, as its own comment says. The seen propagation, the zext_dst
computation via bpf_insn_def32(), the non_stack_access transfer keyed on
memcmp() against the original insn, the last-slot bpf_is_mem_insn()
marking and the move of indirect_target to the first slot are all
duplicated.
Could adjust_insn_aux_data() and patch_list_adj_aux() share a small helper
that fills 'len' aux slots from the insns that replaced 'orig' (taking the
slot base, the patch insns and the mode), so the aux propagation rules live
in one place?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36845462631
