Re: [PATCH] bpf-next: Prevent out of bound buffer write in __bpf_get_stack

["Lecomte, Arnaud" <contact@arnaud-lcm.com>]

On 09/01/2026 19:05, Andrii Nakryiko wrote:
> On Wed, Jan 7, 2026 at 10:35 AM Lecomte, Arnaud <contact@arnaud-lcm.com> wrote:
>>
>> On 07/01/2026 19:08, Lecomte, Arnaud wrote:
>>> On 06/01/2026 01:51, Andrii Nakryiko wrote:
>>>> On Sun, Jan 4, 2026 at 12:52 PM Arnaud Lecomte
>>>> <contact@arnaud-lcm.com> wrote:
>>>>> Syzkaller reported a KASAN slab-out-of-bounds write in
>>>>> __bpf_get_stack()
>>>>> during stack trace copying.
>>>>>
>>>>> The issue occurs when: the callchain entry (stored as a per-cpu
>>>>> variable)
>>>>> grow between collection and buffer copy, causing it to exceed the
>>>>> initially
>>>>> calculated buffer size based on max_depth.
>>>>>
>>>>> The callchain collection intentionally avoids locking for performance
>>>>> reasons, but this creates a window where concurrent modifications can
>>>>> occur during the copy operation.
>>>>>
>>>>> To prevent this from happening, we clamp the trace len to the max
>>>>> depth initially calculated with the buffer size and the size of
>>>>> a trace.
>>>>>
>>>>> Reported-by: syzbot+d1b7fa1092def3628bd7@syzkaller.appspotmail.com
>>>>> Closes:
>>>>> https://lore.kernel.org/all/691231dc.a70a0220.22f260.0101.GAE@google.com/T/
>>>>> Fixes: e17d62fedd10 ("bpf: Refactor stack map trace depth
>>>>> calculation into helper function")
>>>>> Tested-by: syzbot+d1b7fa1092def3628bd7@syzkaller.appspotmail.com
>>>>> Cc: Brahmajit Das <listout@listout.xyz>
>>>>> Signed-off-by: Arnaud Lecomte <contact@arnaud-lcm.com>
>>>>> ---
>>>>> Thanks Brahmajit Das for the initial fix he proposed that I tweaked
>>>>> with the correct justification and a better implementation in my
>>>>> opinion.
>>>>> ---
>>>>>    kernel/bpf/stackmap.c | 4 ++--
>>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
>>>>> index da3d328f5c15..e56752a9a891 100644
>>>>> --- a/kernel/bpf/stackmap.c
>>>>> +++ b/kernel/bpf/stackmap.c
>>>>> @@ -465,7 +465,6 @@ static long __bpf_get_stack(struct pt_regs
>>>>> *regs, struct task_struct *task,
>>>>>
>>>>>           if (trace_in) {
>>>>>                   trace = trace_in;
>>>>> -               trace->nr = min_t(u32, trace->nr, max_depth);
>>>>>           } else if (kernel && task) {
>>>>>                   trace = get_callchain_entry_for_task(task, max_depth);
>>>>>           } else {
>>>>> @@ -479,7 +478,8 @@ static long __bpf_get_stack(struct pt_regs
>>>>> *regs, struct task_struct *task,
>>>>>                   goto err_fault;
>>>>>           }
>>>>>
>>>>> -       trace_nr = trace->nr - skip;
>>>>> +       trace_nr = min(trace->nr, max_depth);
>>>> there is `trace->nr < skip` check right above, should it be moved here
>>>> and done against adjusted trace_nr (but before we subtract skip, of
>>>> course)?
>>> We could indeed be more proactive on the clamping even-though I would
>>>   say it does not fundamentally change anything in my opinion.
>>> Happy to raise a new rev.
>> Nvm, this is not really possible as we are checking that the trace is
>> not NULL.
>> Moving it above could lead to a NULL dereference.
> ok, so what are we doing then?
>
> if (unlikely(!trace)) { ... }
>
> trace_nr = min(trace->nr, max_depth);
>
> if (trace->nr < skip) { ... }
>
> trace_nr = trace->nr - skip;
>
>
> (which is what I proposed, or am I still missing why this shouldn't be
> done like that?)

I think it doesn't really bring any value to adopt this split check.
The underlying problem is the lack of locking mechanism on
the trace structure. I will try to find a workaround minimizing
it's performance impact. I think this is an interesting problem
actually. Will get back to you soon !

> pw-bot: cr
>
>
>
>>>>> +       trace_nr = trace_nr - skip;
>>>>>           copy_len = trace_nr * elem_size;
>>>>>
>>>>>           ips = trace->ip + skip;
>>>>> --
>>>>> 2.43.0
>>>>>
>>> Thanks for the review !
>>> Arnaud

Thanks, Arnaud