* [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset()
@ 2026-06-09 21:07 Ihor Solodrai
2026-06-09 21:20 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Ihor Solodrai @ 2026-06-09 21:07 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu
Cc: bpf, kernel-team
This patch is a follow up to recent implementation of
stack_map_get_build_id_offset_sleepable() [1].
stack_map_get_build_id_offset() and its sleepable variant each cached
only the last successfully resolved VMA, with separate bookkeeping in
each function. A run of IPs in a VMA with no usable build ID will
repeat the lookup for every frame: find_vma() in the non-sleepable
path, a VMA lock and a blocking build_id_parse_file() in the sleepable.
Factor the per-call cache into a shared struct stack_map_build_id_cache
with two independent slots [2][3], used by both functions:
* resolved - last VMA that produced a build ID (file, build_id and
range), reused to skip the lookup and the parse;
* unresolved - last VMA with no usable build ID (range only), reused to
emit a raw IP without another lookup or parse.
Keeping the slots independent means a build-ID-less VMA no longer evicts
the last resolved build ID, so a trace alternating between a binary and a
region without one stops re-resolving the binary on every return.
The shared lookup tests [vm_start, vm_end), matching the sleepable path;
the non-sleepable path previously reused a build ID for ip == vm_end
(range_in_vma() is inclusive) and now re-resolves it correctly.
[1] https://lore.kernel.org/bpf/20260525223948.1920986-1-ihor.solodrai@linux.dev/
[2] https://lore.kernel.org/bpf/CAEf4Bza2fRDGhLQoPE-EzM7F34xaEJfi5Exmxb-iWVUN3F06=g@mail.gmail.com/
[3] https://lore.kernel.org/bpf/CAEf4BzZXJFr=1iiVx937ht=4PYQkQHg=eFk810zhMDzXQG3ihw@mail.gmail.com/
Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
kernel/bpf/stackmap.c | 169 +++++++++++++++++++++++++++++-------------
1 file changed, 118 insertions(+), 51 deletions(-)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 77ba03216c09..6a642d6526c0 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -175,6 +175,82 @@ static inline void stack_map_build_id_set_valid(struct bpf_stack_build_id *id,
memcpy(id->build_id, build_id, BUILD_ID_SIZE_MAX);
}
+/*
+ * Per stack_map_get_build_id_offset() call cache of the last VMA with a build ID
+ * resolved and the last VMA with no usable build ID.
+ * Adjacent stack frames tend to land in the same VMA (deep recursion) or the same
+ * backing file (the main executable and a few shared libraries), so caching the
+ * last result of each kind lets us skip the VMA lookup and, for the resolved slot,
+ * the build ID parse. Tracking the two separately means a build-ID-less VMA doesn't
+ * evict the last resolved build ID. A zero vm_end marks a slot as empty.
+ * resolved.build_id aliases the id_offs[] entry.
+ */
+struct stack_map_build_id_cache {
+ struct {
+ struct file *file; /* pinned in the sleepable path; NULL otherwise */
+ const unsigned char *build_id;
+ unsigned long vm_start;
+ unsigned long vm_end;
+ unsigned long vm_pgoff;
+ } resolved;
+ struct {
+ unsigned long vm_start;
+ unsigned long vm_end;
+ } unresolved;
+};
+
+/*
+ * Fill @id from a cached range covering @ip. On a hit this writes @id (resolved
+ * range -> build ID + offset, unresolved range -> raw ip) and returns 0; on a
+ * miss it leaves @id untouched and returns -ENOENT.
+ */
+static int stack_map_build_id_set_from_cache(struct stack_map_build_id_cache *cache,
+ struct bpf_stack_build_id *id, u64 ip)
+{
+ if (cache->resolved.vm_end && ip >= cache->resolved.vm_start &&
+ ip < cache->resolved.vm_end) {
+ u64 offset = stack_map_build_id_offset(cache->resolved.vm_pgoff,
+ cache->resolved.vm_start, ip);
+ stack_map_build_id_set_valid(id, offset, cache->resolved.build_id);
+ return 0;
+ }
+ if (cache->unresolved.vm_end && ip >= cache->unresolved.vm_start &&
+ ip < cache->unresolved.vm_end) {
+ stack_map_build_id_set_ip(id);
+ return 0;
+ }
+ return -ENOENT;
+}
+
+/*
+ * Record @vma's build ID as the last resolved one. @file is the pinned backing
+ * file in the sleepable path (released when evicted), or NULL otherwise.
+ */
+static void stack_map_build_id_cache_set_resolved(struct stack_map_build_id_cache *cache,
+ struct file *file,
+ const unsigned char *build_id,
+ unsigned long vm_start,
+ unsigned long vm_end,
+ unsigned long vm_pgoff)
+{
+ if (cache->resolved.file)
+ fput(cache->resolved.file);
+ cache->resolved.file = file;
+ cache->resolved.build_id = build_id;
+ cache->resolved.vm_start = vm_start;
+ cache->resolved.vm_end = vm_end;
+ cache->resolved.vm_pgoff = vm_pgoff;
+}
+
+/* Record [vm_start, vm_end) as a range with no usable build ID. */
+static void stack_map_build_id_cache_set_unresolved(struct stack_map_build_id_cache *cache,
+ unsigned long vm_start,
+ unsigned long vm_end)
+{
+ cache->unresolved.vm_start = vm_start;
+ cache->unresolved.vm_end = vm_end;
+}
+
struct stack_map_vma_lock {
struct vm_area_struct *vma;
struct mm_struct *mm;
@@ -246,13 +322,7 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
{
struct mm_struct *mm = current->mm;
struct stack_map_vma_lock lock = { .mm = mm };
- struct {
- struct file *file;
- const unsigned char *build_id;
- unsigned long vm_start;
- unsigned long vm_end;
- unsigned long vm_pgoff;
- } cache = {};
+ struct stack_map_build_id_cache cache = {};
unsigned long vm_pgoff, vm_start, vm_end;
struct vm_area_struct *vma;
struct file *file;
@@ -262,44 +332,43 @@ 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);
- /*
- * Range cache fast path: if ip falls within the previously
- * resolved VMA range, reuse the cache build_id without
- * re-acquiring the VMA lock.
- */
- if (cache.build_id && ip >= cache.vm_start && ip < cache.vm_end) {
- offset = stack_map_build_id_offset(cache.vm_pgoff, cache.vm_start, ip);
- stack_map_build_id_set_valid(&id_offs[i], offset, cache.build_id);
+ if (!stack_map_build_id_set_from_cache(&cache, &id_offs[i], ip))
continue;
- }
vma = stack_map_lock_vma(&lock, ip);
if (!vma) {
stack_map_build_id_set_ip(&id_offs[i]);
continue;
}
+
+ vm_pgoff = vma->vm_pgoff;
+ vm_start = vma->vm_start;
+ vm_end = vma->vm_end;
+
if (vma_is_anonymous(vma) || !vma->vm_file) {
- stack_map_build_id_set_ip(&id_offs[i]);
stack_map_unlock_vma(&lock);
+ stack_map_build_id_set_ip(&id_offs[i]);
+ stack_map_build_id_cache_set_unresolved(&cache, vm_start, vm_end);
continue;
}
file = vma->vm_file;
- vm_pgoff = vma->vm_pgoff;
- vm_start = vma->vm_start;
- vm_end = vma->vm_end;
offset = stack_map_build_id_offset(vm_pgoff, vm_start, ip);
/*
- * Same backing file as previous (e.g. different VMAs
- * of the same ELF binary). Reuse the cache build_id.
+ * Same backing file as the last resolved VMA (e.g. different
+ * VMAs of the same ELF binary). Reuse its build_id without
+ * re-parsing. file is non-NULL here (anonymous / no-file VMAs
+ * were handled above), so cache.resolved.file == file already
+ * implies a populated resolved slot.
*/
- if (file == cache.file) {
+ if (file == cache.resolved.file) {
stack_map_unlock_vma(&lock);
- stack_map_build_id_set_valid(&id_offs[i], offset, cache.build_id);
- cache.vm_start = vm_start;
- cache.vm_end = vm_end;
- cache.vm_pgoff = vm_pgoff;
+ stack_map_build_id_set_valid(&id_offs[i], offset,
+ cache.resolved.build_id);
+ cache.resolved.vm_start = vm_start;
+ cache.resolved.vm_end = vm_end;
+ cache.resolved.vm_pgoff = vm_pgoff;
continue;
}
@@ -310,21 +379,17 @@ static void stack_map_get_build_id_offset_sleepable(struct bpf_stack_build_id *i
if (build_id_parse_file(file, id_offs[i].build_id, NULL)) {
stack_map_build_id_set_ip(&id_offs[i]);
fput(file);
+ stack_map_build_id_cache_set_unresolved(&cache, vm_start, vm_end);
continue;
}
stack_map_build_id_set_valid(&id_offs[i], offset, id_offs[i].build_id);
- if (cache.file)
- fput(cache.file);
- cache.file = file;
- cache.build_id = id_offs[i].build_id;
- cache.vm_start = vm_start;
- cache.vm_end = vm_end;
- cache.vm_pgoff = vm_pgoff;
+ stack_map_build_id_cache_set_resolved(&cache, file, id_offs[i].build_id,
+ vm_start, vm_end, vm_pgoff);
}
- if (cache.file)
- fput(cache.file);
+ if (cache.resolved.file)
+ fput(cache.resolved.file);
}
/*
@@ -343,8 +408,8 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
struct mmap_unlock_irq_work *work = NULL;
bool irq_work_busy = bpf_mmap_unlock_get_irq_work(&work);
bool has_user_ctx = user && current && current->mm;
- struct vm_area_struct *vma, *prev_vma = NULL;
- const unsigned char *prev_build_id = NULL;
+ struct stack_map_build_id_cache cache = {};
+ struct vm_area_struct *vma;
int i;
if (may_fault && has_user_ctx) {
@@ -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)) {
- /* per entry fall back to ips */
+ /* per entry fall back to ips; cache build-ID-less range */
stack_map_build_id_set_ip(&id_offs[i]);
- prev_vma = vma;
- prev_build_id = NULL;
+ if (vma)
+ stack_map_build_id_cache_set_unresolved(&cache,
+ vma->vm_start, vma->vm_end);
continue;
}
- offset = stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip);
- stack_map_build_id_set_valid(&id_offs[i], offset, id_offs[i].build_id);
- prev_vma = vma;
- prev_build_id = id_offs[i].build_id;
+ /* mmap_lock is held for the whole loop, so the cached VMA
+ * fields stay valid; no file pinning is needed here.
+ */
+ stack_map_build_id_set_valid(&id_offs[i],
+ stack_map_build_id_offset(vma->vm_pgoff, vma->vm_start, ip),
+ id_offs[i].build_id);
+ stack_map_build_id_cache_set_resolved(&cache, NULL, id_offs[i].build_id,
+ vma->vm_start, vma->vm_end,
+ vma->vm_pgoff);
}
bpf_mmap_unlock_mm(work, current->mm);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset()
2026-06-09 21:07 [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset() Ihor Solodrai
@ 2026-06-09 21:20 ` sashiko-bot
2026-06-09 21:40 ` Alexei Starovoitov
2026-06-09 21:42 ` bot+bpf-ci
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-09 21:20 UTC (permalink / raw)
To: Ihor Solodrai; +Cc: bpf
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset()
2026-06-09 21:07 [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset() Ihor Solodrai
2026-06-09 21:20 ` sashiko-bot
@ 2026-06-09 21:40 ` Alexei Starovoitov
2026-06-09 21:42 ` bot+bpf-ci
2 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2026-06-09 21:40 UTC (permalink / raw)
To: Ihor Solodrai, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Eduard Zingerman, Kumar Kartikeya Dwivedi,
Song Liu
Cc: bpf, kernel-team
On Tue Jun 9, 2026 at 2:07 PM PDT, Ihor Solodrai wrote:
> This patch is a follow up to recent implementation of
> stack_map_get_build_id_offset_sleepable() [1].
>
> stack_map_get_build_id_offset() and its sleepable variant each cached
> only the last successfully resolved VMA, with separate bookkeeping in
> each function. A run of IPs in a VMA with no usable build ID will
> repeat the lookup for every frame: find_vma() in the non-sleepable
> path, a VMA lock and a blocking build_id_parse_file() in the sleepable.
>
> Factor the per-call cache into a shared struct stack_map_build_id_cache
> with two independent slots [2][3], used by both functions:
>
> * resolved - last VMA that produced a build ID (file, build_id and
> range), reused to skip the lookup and the parse;
> * unresolved - last VMA with no usable build ID (range only), reused to
> emit a raw IP without another lookup or parse.
>
> Keeping the slots independent means a build-ID-less VMA no longer evicts
> the last resolved build ID, so a trace alternating between a binary and a
> region without one stops re-resolving the binary on every return.
>
> The shared lookup tests [vm_start, vm_end), matching the sleepable path;
> the non-sleepable path previously reused a build ID for ip == vm_end
> (range_in_vma() is inclusive) and now re-resolves it correctly.
>
> [1] https://lore.kernel.org/bpf/20260525223948.1920986-1-ihor.solodrai@linux.dev/
> [2] https://lore.kernel.org/bpf/CAEf4Bza2fRDGhLQoPE-EzM7F34xaEJfi5Exmxb-iWVUN3F06=g@mail.gmail.com/
> [3] https://lore.kernel.org/bpf/CAEf4BzZXJFr=1iiVx937ht=4PYQkQHg=eFk810zhMDzXQG3ihw@mail.gmail.com/
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---
> kernel/bpf/stackmap.c | 169 +++++++++++++++++++++++++++++-------------
> 1 file changed, 118 insertions(+), 51 deletions(-)
>
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 77ba03216c09..6a642d6526c0 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -175,6 +175,82 @@ static inline void stack_map_build_id_set_valid(struct bpf_stack_build_id *id,
> memcpy(id->build_id, build_id, BUILD_ID_SIZE_MAX);
> }
>
> +/*
> + * Per stack_map_get_build_id_offset() call cache of the last VMA with a build ID
> + * resolved and the last VMA with no usable build ID.
> + * Adjacent stack frames tend to land in the same VMA (deep recursion) or the same
> + * backing file (the main executable and a few shared libraries), so caching the
> + * last result of each kind lets us skip the VMA lookup and, for the resolved slot,
> + * the build ID parse. Tracking the two separately means a build-ID-less VMA doesn't
> + * evict the last resolved build ID. A zero vm_end marks a slot as empty.
> + * resolved.build_id aliases the id_offs[] entry.
> + */
> +struct stack_map_build_id_cache {
> + struct {
> + struct file *file; /* pinned in the sleepable path; NULL otherwise */
> + const unsigned char *build_id;
> + unsigned long vm_start;
> + unsigned long vm_end;
> + unsigned long vm_pgoff;
> + } resolved;
> + struct {
> + unsigned long vm_start;
> + unsigned long vm_end;
> + } unresolved;
> +};
> +
> +/*
> + * Fill @id from a cached range covering @ip. On a hit this writes @id (resolved
> + * range -> build ID + offset, unresolved range -> raw ip) and returns 0; on a
> + * miss it leaves @id untouched and returns -ENOENT.
> + */
> +static int stack_map_build_id_set_from_cache(struct stack_map_build_id_cache *cache,
> + struct bpf_stack_build_id *id, u64 ip)
> +{
> + if (cache->resolved.vm_end && ip >= cache->resolved.vm_start &&
> + ip < cache->resolved.vm_end) {
> + u64 offset = stack_map_build_id_offset(cache->resolved.vm_pgoff,
> + cache->resolved.vm_start, ip);
empty line is needed or move 'u64 offset' out.
> + stack_map_build_id_set_valid(id, offset, cache->resolved.build_id);
> + return 0;
> + }
> + if (cache->unresolved.vm_end && ip >= cache->unresolved.vm_start &&
> + ip < cache->unresolved.vm_end) {
> + stack_map_build_id_set_ip(id);
> + return 0;
> + }
Please declaude it.
All the derefs can be shortened and more human readable.
For example: give struct a name:
struct resolved_vma *res = &cache->resolved;
struct unresolved_vma *unres = &cache->unresolved;
if (rev->vm_end && ip >= res->vm_start
if (unres->vm_end && ...)
or any other option. Just so that code doesn't scream "claude's slop".
pw-bot: cr
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset()
2026-06-09 21:07 [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset() Ihor Solodrai
2026-06-09 21:20 ` sashiko-bot
2026-06-09 21:40 ` Alexei Starovoitov
@ 2026-06-09 21:42 ` bot+bpf-ci
2 siblings, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-06-09 21:42 UTC (permalink / raw)
To: ihor.solodrai, ast, andrii, daniel, eddyz87, memxor, song
Cc: bpf, kernel-team, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3224 bytes --]
> bpf: Better build_id caching in stack_map_get_build_id_offset()
>
> This patch is a follow up to recent implementation of
> stack_map_get_build_id_offset_sleepable() [1].
> stack_map_get_build_id_offset() and its sleepable variant each cached
> only the last successfully resolved VMA, with separate bookkeeping in
> each function. A run of IPs in a VMA with no usable build ID will
> repeat the lookup for every frame: find_vma() in the non-sleepable
> path, a VMA lock and a blocking build_id_parse_file() in the sleepable.
> Factor the per-call cache into a shared struct stack_map_build_id_cache
> with two independent slots [2][3], used by both functions:
> * resolved - last VMA that produced a build ID (file, build_id and
> range), reused to skip the lookup and the parse;
> * unresolved - last VMA with no usable build ID (range only), reused to
> emit a raw IP without another lookup or parse.
> Keeping the slots independent means a build-ID-less VMA no longer evicts
> the last resolved build ID, so a trace alternating between a binary and a
> region without one stops re-resolving the binary on every return.
> The shared lookup tests [vm_start, vm_end), matching the sleepable path;
> the non-sleepable path previously reused a build ID for ip == vm_end
> (range_in_vma() is inclusive) and now re-resolves it correctly.
> [1] https://lore.kernel.org/bpf/20260525223948.1920986-1-ihor.solodrai@linux.dev/
> [2] https://lore.kernel.org/bpf/CAEf4Bza2fRDGhLQoPE-EzM7F34xaEJfi5Exmxb-iWVUN3F06=g@mail.gmail.com/
> [3] https://lore.kernel.org/bpf/CAEf4BzZXJFr=1iiVx937ht=4PYQkQHg=eFk810zhMDzXQG3ihw@mail.gmail.com/
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
This commit fixes a user-visible correctness bug in
stack_map_get_build_id_offset() but does not carry a Fixes: tag. The
commit message notes:
> the non-sleepable path previously reused a build ID for ip == vm_end
> (range_in_vma() is inclusive) and now re-resolves it correctly.
The buggy code is the range cache check:
if (prev_build_id && range_in_vma(prev_vma, ip, ip))
range_in_vma() is inclusive of vm_end, so an IP equal to
prev_vma->vm_end (which belongs to the next VMA, since ranges are
[vm_start, vm_end)) reuses the previous VMA's build_id and computes a
wrong offset. The change replaces this with an explicit half-open check
(ip >= vm_start && ip < vm_end).
Should this carry a Fixes: tag? The most recent commit in the available
history to establish this prev_vma/prev_build_id range cache (and which
contains the exact range_in_vma(prev_vma, ip, ip) line being replaced)
appears to be:
Fixes: fc99547a8bda ("bpf: Factor out stack_map build ID helpers")
The inclusive range_in_vma() behavior likely predates fc99547a8bda,
which refactored the cache and added the prev_build_id guard but
inherited the inclusive check, so the true origin of the off-by-one may
be older than this.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27236707090
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-09 21:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-09 21:07 [PATCH bpf-next v1] bpf: Better build_id caching in stack_map_get_build_id_offset() Ihor Solodrai
2026-06-09 21:20 ` sashiko-bot
2026-06-09 21:40 ` Alexei Starovoitov
2026-06-09 21:42 ` bot+bpf-ci
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox