On Tue, 2024-08-27 at 12:48 -0700, Martin KaFai Lau wrote:
> From: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
>
> This patch adds a .gen_epilogue to the bpf_verifier_ops. It is similar
> to the existing .gen_prologue. Instead of allowing a subsystem
> to run code at the beginning of a bpf prog, it allows the subsystem
> to run code just before the bpf prog exit.
>
> One of the use case is to allow the upcoming bpf qdisc to ensure that
> the skb->dev is the same as the qdisc->dev_queue->dev. The bpf qdisc
> struct_ops implementation could either fix it up or drop the skb.
> Another use case could be in bpf_tcp_ca.c to enforce snd_cwnd
> has sane value (e.g. non zero).
>
> The epilogue can do the useful thing (like checking skb->dev) if it
> can access the bpf prog's ctx. Unlike prologue, r1 may not hold the
> ctx pointer. This patch saves the r1 in the stack if the .gen_epilogue
> has returned some instructions in the "epilogue_buf".
>
> The existing .gen_prologue is done in convert_ctx_accesses().
> The new .gen_epilogue is done in the convert_ctx_accesses() also.
> When it sees the (BPF_JMP | BPF_EXIT) instruction, it will be patched
> with the earlier generated "epilogue_buf". The epilogue patching is
> only done for the main prog.
>
> Only one epilogue will be patched to the main program. When the
> bpf prog has multiple BPF_EXIT instructions, a BPF_JA is used
> to goto the earlier patched epilogue. Majority of the archs
> support (BPF_JMP32 | BPF_JA): x86, arm, s390, risv64, loongarch,
> powerpc and arc. This patch keeps it simple and always
> use (BPF_JMP32 | BPF_JA).
>
> Signed-off-by: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
> ---
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -19740,6 +19764,26 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> insn->code = BPF_STX | BPF_PROBE_ATOMIC | BPF_SIZE(insn->code);
> env->prog->aux->num_exentries++;
> continue;
> + } else if (insn->code == (BPF_JMP | BPF_EXIT) &&
> + epilogue_cnt &&
> + i + delta < subprogs[1].start) {
> + /* Generate epilogue for the main prog */
> + if (epilogue_idx) {
> + /* jump back to the earlier generated epilogue */
> + insn_buf[0] = BPF_JMP32_IMM(BPF_JA, 0,
> + epilogue_idx - i - delta - 1, 0);
Nit: maybe add BPF_GOTOL macro or mention that this is a 'gotol' instruction in the comment?
(this is how it is called in llvm).
> + cnt = 1;
> + } else {
> + memcpy(insn_buf, epilogue_buf,
> + epilogue_cnt * sizeof(*epilogue_buf));
> + cnt = epilogue_cnt;
> + /* epilogue_idx cannot be 0. It must have at
> + * least one ctx ptr saving insn before the
> + * epilogue.
> + */
> + epilogue_idx = i + delta;
> + }
> + goto patch_insn_buf;
> } else {
> continue;
> }
[...]
