On 2026/2/25 00:57, Alexei Starovoitov wrote:
> On Tue, Feb 24, 2026 at 7:41 AM Leon Hwang <[email protected]> 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 <[email protected]>
>> ---
>> 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
