Re: [PATCH bpf v3 3/3] bpf: Cache build IDs in sleepable stackmap path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect VMA classification using `!vma->vm_file` instead of `vma_is_anonymous()`.
--

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

bpf: Cache build IDs in sleepable stackmap path

This commit introduces a cache for the last successfully parsed build ID,
VMA range, and backing file in the sleepable stackmap path. This avoids
repeated VMA locking and file parsing for adjacent IPs.

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index c1e96df360c3f..318ce9ed0dd5c 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
[ ... ]
> @@ -226,13 +226,34 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
[ ... ]
>  	for (u32 i = 0; i < trace_nr; i++) {
>  		ip = READ_ONCE(id_offs[i].ip);
[ ... ]
>  		vma = stack_map_lock_vma(&lock, ip);
>  		if (!vma || !vma->vm_file) {

While this specific check was not introduced by this patch, I notice the
code relies on !vma->vm_file to determine if a VMA is file-backed.

Can this cause issues with anonymous VMAs that still have a backing file?

For example, private mappings of /dev/zero or driver memory can have a
non-NULL vma->vm_file but are still considered anonymous.

If the stackmap path proceeds to parse the ELF build ID from these files,
could it trigger unintended side effects, such as consuming FIFO data or
altering hardware state when reading from character devices?

Would it be safer to check vma_is_anonymous(vma) here to properly classify
the VMA?

>  			stack_map_build_id_set_ip(&id_offs[i]);
>  			stack_map_unlock_vma(&lock);
>  			continue;
>  		}

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