Re: [PATCH bpf-next v9 2/6] bpf: Add bpf_prog_run_array_sleepable()

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 4/13/26 5:25 PM, Alexei Starovoitov wrote:
> On Mon, Apr 13, 2026 at 5:55 AM Mykyta Yatsenko
> <mykyta.yatsenko5@gmail.com> wrote:
>>
>>
>>
>> On 4/10/26 11:55 PM, Alexei Starovoitov wrote:
>>> On Fri, Apr 10, 2026 at 10:09 AM Mykyta Yatsenko
>>> <mykyta.yatsenko5@gmail.com> wrote:
>>>>
>>>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>>>
>>>> Add bpf_prog_run_array_sleepable() for running BPF program arrays
>>>> on faultable tracepoints. Unlike bpf_prog_run_array_uprobe(), it
>>>> includes per-program recursion checking for private stack safety
>>>> and hardcodes is_uprobe to false.
>>>>
>>>> Keep bpf_prog_run_array_uprobe() unchanged for uprobe callers.
>>>>
>>>> Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>>>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>>>> ---
>>>>    include/linux/bpf.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 50 insertions(+)
>>>>
>>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>>> index 0136a108d083..4e166accab35 100644
>>>> --- a/include/linux/bpf.h
>>>> +++ b/include/linux/bpf.h
>>>> @@ -3077,6 +3077,56 @@ void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr);
>>>>    void bpf_dynptr_set_rdonly(struct bpf_dynptr_kern *ptr);
>>>>    void bpf_prog_report_arena_violation(bool write, unsigned long addr, unsigned long fault_ip);
>>>>
>>>> +static __always_inline u32
>>>> +bpf_prog_run_array_sleepable(const struct bpf_prog_array *array,
>>>> +                            const void *ctx, bpf_prog_run_fn run_prog)
>>>> +{
>>>> +       const struct bpf_prog_array_item *item;
>>>> +       struct bpf_prog *prog;
>>>> +       struct bpf_run_ctx *old_run_ctx;
>>>> +       struct bpf_trace_run_ctx run_ctx;
>>>> +       u32 ret = 1;
>>>> +
>>>> +       might_fault();
>>>> +       RCU_LOCKDEP_WARN(!rcu_read_lock_trace_held(), "no rcu lock held");
>>>> +
>>>> +       if (unlikely(!array))
>>>> +               return ret;
>>>> +
>>>> +       migrate_disable();
>>>> +
>>>> +       run_ctx.is_uprobe = false;
>>>> +
>>>> +       old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
>>>> +       item = &array->items[0];
>>>> +       while ((prog = READ_ONCE(item->prog))) {
>>>> +               if (!prog->sleepable)
>>>> +                       rcu_read_lock();
>>>> +
>>>> +               /* Per-prog recursion check to enable private stack. */
>>>> +               if (unlikely(!bpf_prog_get_recursion_context(prog))) {
>>>
>>> from sashiko
>>>
>>>> + old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
>>>> + item = &array->items[0];
>>>> + while ((prog = READ_ONCE(item->prog))) {
>>>> + if (!prog->sleepable)
>>>> + rcu_read_lock();
>>>> +
>>>> + /* Per-prog recursion check to enable private stack. */
>>>> + if (unlikely(!bpf_prog_get_recursion_context(prog))) {
>>>
>>> Can this cause a panic by dereferencing dummy_bpf_prog.prog.active?
>>> When a program is detached from a BPF array and memory allocation for the new
>>> array fails, bpf_prog_array_delete_safe() replaces the detached program with
>>> &dummy_bpf_prog.prog as a fallback.
>>> Because dummy_bpf_prog is a statically allocated placeholder, its prog.active
>>> field is uninitialized (NULL).
>>> If prog is dummy_bpf_prog.prog, bpf_prog_get_recursion_context() will
>>> dereference prog->active as a per-CPU pointer, accessing offset 0 in the
>>> per-CPU area and causing memory corruption or a panic.
>>> Should there be an explicit check to skip dummy_bpf_prog.prog here?
>>
>> Looks like a real issue, thanks. I think the best solution is to add
>> valid `prog->active` field for the dummy_bpf_prog.prog so we don't need
>> to maintain special branches and can rely on the being a valid bpf_prog.
> 
> No. Don't copy paste from claude. Ask it to do the home work.
> This was already discussed on the list.
> I mean 'oh it's NULL let's add it'. It was similar issue elsewhere
> and solved differently.

Actually claude suggested to simply check if prog == 
dummy_bpf_prog.prog, similarly to how it's done in kernel/bpf/core.c 
(for example function bpf_prog_array_copy_core()), but to me adding a 
valid `active` field sounds like a more future proof solution.