[PATCH bpf-next 1/4] bpf: Document the aux data rules of patched instructions

[Qiliang Yuan]

The aux data of a patch derives from the aux data of the insn that it
replaces: zext_dst is always recomputed, the new insns inherit the seen
count, and a memory accessing insn that is not a copy of the patched
one is marked for non-stack access.

patch_list_adj_aux() and the in-place path of bpf_patch_list_add()
implement these rules without a word about them. The comments that
explained them went away with adjust_insn_aux_data().

Add the comments back next to the code that implements the rules.

Reported-by: Alexei Starovoitov <ast@xxxxxxxxxx>
Closes: https://lore.kernel.org/bpf/DMKHZS5B5CNO.186F6YH1T6YU4@xxxxxxxxx/
Signed-off-by: Qiliang Yuan <odys.yuan@xxxxxxxxx>
---
 kernel/bpf/fixups.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index ead78713e04f4..6d186a096634c 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -203,7 +203,11 @@ struct bpf_insn *bpf_patch_list_add(struct bpf_verifier_env *env, u32 off,
 
 		*insn = *patch;
 		env->insn_aux_data[off].zext_dst = bpf_insn_def32(env->prog, insn) >= 0;
-		/* a new insn may access memory, systematically mark it non-stack */
+		/*
+		 * A non-memory accessing insn could have been replaced by a
+		 * memory accessing insn, systematically mark it for non-stack
+		 * access.
+		 */
 		env->insn_aux_data[off].non_stack_access = bpf_is_mem_insn(insn);
 		return insn;
 	}
@@ -270,14 +274,21 @@ static void patch_list_adj_aux(struct bpf_prog *prog, struct bpf_insn_patch *p,
 	u32 i;
 
 	*last = *old;
+	/* Recompute zext_dst: the last insn of the patch need not be the patched one. */
 	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++) {
+		/* Expand the seen count of the patched insn to the patch. */
 		slot[i].seen = last->seen;
 		/* messages name the insn of the original program */
 		slot[i].orig_idx = last->orig_idx;
 		slot[i].zext_dst = bpf_insn_def32(prog, &patch[i]) >= 0;
+		/*
+		 * A copy of the patched insn keeps its marking. Any other
+		 * memory accessing insn is new, systematically mark it for
+		 * non-stack access.
+		 */
 		if (i >= p->target && !memcmp(&patch[i], orig, sizeof(*orig))) {
 			slot[i].non_stack_access = last->non_stack_access;
 			last->non_stack_access = false;
@@ -286,6 +297,11 @@ static void patch_list_adj_aux(struct bpf_prog *prog, struct bpf_insn_patch *p,
 		}
 	}
 
+	/*
+	 * The last insn could be a newly generated BPF_ST/BPF_LDX/BPF_STX,
+	 * systematically mark it for non-stack access if it is not the patched
+	 * insn, otherwise keep the original marking.
+	 */
 	if (bpf_is_mem_insn(&patch[p->len - 1]) &&
 	    memcmp(&patch[p->len - 1], orig, sizeof(*orig)))
 		last->non_stack_access = true;

-- 
2.43.0