Re: [PATCH bpf-next v3 3/6] bpf: Disallow !kprobe_write_ctx progs tail-calling kprobe_write_ctx progs

[Leon Hwang]

On 11/3/26 01:23, Kumar Kartikeya Dwivedi wrote:
> On Tue, 3 Mar 2026 at 17:04, <bot+bpf...@kernel.org> wrote:
>>
>>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>>> --- a/kernel/bpf/core.c
>>> +++ b/kernel/bpf/core.c
>>
>> [ ... ]
>>
>>> +     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...@mail.gmail.com/
>>
>> 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.

Thanks,
Leon