On Wed, 11 Mar 2026 at 07:08, Leon Hwang [off-list ref] wrote:

On 11/3/26 01:23, Kumar Kartikeya Dwivedi wrote:

quoted

On Tue, 3 Mar 2026 at 17:04, [off-list ref] wrote:

quoted

quoted

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c

[ ... ]

quoted

+     switch (match) {
+     case BPF_MAP_OWNER_MATCH_FOR_INIT:
+             if (owner->kprobe_write_ctx != aux->kprobe_write_ctx)
+                     return false;
+             break;
+
+     case BPF_MAP_OWNER_MATCH_FOR_UPDATE:
+             if (!owner->kprobe_write_ctx && aux->kprobe_write_ctx)
+                     return false;
+             break;
+     }

In the v1 review, Alexei Starovoitov asked for strict conformance
both ways rather than one-directional validation:

    "Don't be fancy. Require strict conformance both ways
    in *all* patches."

https://lore.kernel.org/bpf/CAADnVQ+-V0-1i8_+CYpK7G0CnV-2n8e9Szv43yM3Az197eL_0A@mail.gmail.com/ (local)

The BPF_MAP_OWNER_MATCH_FOR_UPDATE case still uses one-directional
matching, rejecting only !owner->kprobe_write_ctx && aux->kprobe_write_ctx
but allowing the reverse.  Was this an intentional design choice, and
if so, should the changelog note the disagreement?

Let's follow the approach Alexei outlined, while the changes look ok
to me, let's remove the one way check and just do
owner->kprobe_write_ctx != aux->kprobe_write_ctx, same for the other
two checks. With this we can also get rid of this INIT vs UPDATE
distinction.

Other than that I think patches are good, please also test both
directions in the selftest in next respin.

Hi Kumar,

Thanks for your review.

I agreed that both-ways check could simplify the code. But, the UX could
not be great.

Let me explain it with an example.

struct {
        __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
        __uint(max_entries, 1);
        __uint(key_size, sizeof(__u32));
        __uint(value_size, sizeof(__u32));
} jmp_table SEC(".maps");

SEC("?kprobe")
int prog_a(struct pt_regs *regs)
{
        regs->ax = 0;
        bpf_tail_call_static(regs, &jmp_table, 0);
        return 0;
}

SEC("?kprobe")
int prog_b(struct pt_regs *regs)
{
        return 0;
}

prog_a is kprobe_write_ctx.
prog_b is !kprobe_write_ctx. And, it will be added to jmp_table.

With both-ways check, prog_b is required to modify regs. I think it's
too restrictive for users to use prog_b as tail callee.

With one-way-check of this patch, prog_b can be used as tail callee to
keep regs as-is.

This was what I was concerned about, and the reason why I did not follow
Alexei's approach.

I agree but the main question is whether such a case is realistic, are
we going to have write_ctx programs tail calling this way?
Tail calls are already pretty rare, thinking more about it extension
programs are probably also broken wrt checks in this set.
bpf_check_attach_target is doing none of these things for
prog_extension = true. Nobody reported a problem, so I doubt anyone is
hitting this.
It probably also needs to be fixed.
Since you noticed it, we should close the gap conservatively for now,
and wait for a real use case to pop up before enabling this one-way.

Thanks,
Leon