Re: [PATCH bpf-next 2/8] bpf: Disallow !kprobe_write_ctx progs tail-calling kprobe_write_ctx progs

[Leon Hwang <leon.hwang@linux.dev>]

On 2026/2/25 00:57, Alexei Starovoitov wrote:
> On Tue, Feb 24, 2026 at 7:41 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> uprobe programs that can modify pt_regs require different runtime
>> assumptions than pt_regs-read-only uprobe programs. Mixing both in
>> one prog_array can make owner expectations diverge from callee behavior.
>>
>> Reject the combination of !kprobe_write_ctx progs with kprobe_write_ctx
>> progs in __bpf_prog_map_compatible() to address the issue.
>>
>> Fixes: 7384893d970e ("bpf: Allow uprobe program to change context registers")
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  include/linux/bpf.h | 7 ++++---
>>  kernel/bpf/core.c   | 3 +++
>>  2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index b78b53198a2e..2a2f6448a5fb 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -285,9 +285,10 @@ struct bpf_list_node_kern {
>>   */
>>  struct bpf_map_owner {
>>         enum bpf_prog_type type;
>> -       bool jited;
>> -       bool xdp_has_frags;
>> -       bool sleepable;
>> +       u32 jited:1,
>> +           xdp_has_frags:1,
>> +           sleepable:1,
>> +           kprobe_write_ctx:1;
> 
> Don't you see how much churn you're adding this way?
> Every patch has to touch two lines instead of one.
> Use
> u32 jited:1;
> u32 xdp_has_frags:1;
> 

Ack.

> also the bot is correct on patch 2 and 3.

Agreed.

Here's a concrete example that breaks the constraint:

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)
{
	bpf_tail_call_static(regs, &jmp_table, 0);
	return 0;
}

The jmp_table is shared between prog_a and prog_b.

The constraint can be broken by:

* Load prog_a first.
  At this point, owner->kprobe_write_ctx=true.
* Load prog_b next.
  At this point, prog_b passes the prog map compatibility validation.
* Add prog_a to jmp_table.
* Attach prog_b to a kernel function.

When the kernel function runs, the regs will be updated via prog_a.

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

It was to avoid awkward UX.

For example, the tail call from prog_a (kprobe_write_ctx=true) to prog_b
(kprobe_write_ctx=false) should be allowed. If prog_b is required to be
kprobe_write_ctx=true, prog_b will break user's expectation that prog_b
should not always update ctx.

The same awkward UX applies to call_get_func_ip, call_session_cookie,
and call_session_is_return.

I'll find a way to avoid such awkward UX by keeping the one-directional
validation, and to avoid the aforementioned constraint-broken issue at
the same time.

> And your codex selftests are garbage. I don't have other words
> to describe it. They are not testing the actual bug that
> your patches are fixing. Think of what you're doing.
> Asking LLM to write a test for your other patch is not what you
> should be asking it to do. The selftest should be such that
> it proves the unsafety/crash before the fix.
> 

OK. I'll reimplement the selftests by myself in the next revision.

Thanks,
Leon