Re: [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] In the non-sleepable path, stack trace IPs falling into unmapped holes are incorrectly associated with the next adjacent VMA, producing bogus build IDs and out-of-bounds offsets.
--

commit 1e578af5039b6e5b0dc36373f9670148568cdb2a
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

bpf: Better build_id caching in stack_map_get_build_id_offset()

This commit introduces a shared caching mechanism for build IDs between the
sleepable and non-sleepable paths of stack_map_get_build_id_offset(). It
tracks both resolved and unresolved VMAs to avoid repeating lookups and
parses for adjacent stack frames.

Link: https://lore.kernel.org/bpf/20260525223948.1920986-1-ihor.solodrai@linux.dev/
Link: https://lore.kernel.org/bpf/CAEf4Bza2fRDGhLQoPE-EzM7F34xaEJfi5Exmxb-iWVUN3F06=g@mail.gmail.com/
Link: https://lore.kernel.org/bpf/CAEf4BzZXJFr=1iiVx937ht=4PYQkQHg=eFk810zhMDzXQG3ihw@mail.gmail.com/

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 77ba03216c09f..6a642d6526c02 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c

[ ... ]

> @@ -365,27 +430,29 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>  
>  	for (i = 0; i < trace_nr; i++) {
>  		u64 ip = READ_ONCE(id_offs[i].ip);
> -		u64 offset;
>  
> -		if (prev_build_id && range_in_vma(prev_vma, ip, ip)) {
> -			vma = prev_vma;
> -			offset = stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip);
> -			stack_map_build_id_set_valid(&id_offs[i], offset, prev_build_id);
> +		if (!stack_map_build_id_set_from_cache(&cache, &id_offs[i], ip))
>  			continue;
> -		}
> +
>  		vma = find_vma(current->mm, ip);
>  		if (!vma || vma_is_anonymous(vma) ||
>  		    fetch_build_id(vma, id_offs[i].build_id, may_fault)) {

[Severity: High]
This is a pre-existing issue, but does using find_vma() here allow IPs from
unmapped regions to incorrectly bind to the next adjacent VMA?

Since find_vma() returns the first VMA where vm_end > ip without checking
if ip >= vm_start, an unmapped IP could incorrectly fall into this path.
If that happens, computing the file offset later in the loop via
stack_map_build_id_offset() does:

    (vm_pgoff << PAGE_SHIFT) + ip - vm_start;

Could this calculation underflow and produce a massive out-of-bounds offset
if ip < vma->vm_start?

The sleepable variant avoids this because stack_map_lock_vma() uses
vma_lookup() internally, which guarantees the IP falls strictly within the VMA
boundaries. Should this non-sleepable path also be converted to use
vma_lookup(), or explicitly check if ip >= vma->vm_start?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609210749.3448062-1-ihor.solodrai@linux.dev?part=1