mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] perf/x86: Limit branch snapshots to valid entries
@ 2026-10-15 11:46 Sandipan Das
  2026-10-15 11:46 ` [PATCH v2 1/2] perf/x86/amd/lbr: " Sandipan Das
  2026-10-15 11:46 ` [PATCH v2 2/2] perf/x86/intel/lbr: " Sandipan Das
  0 siblings, 2 replies; 6+ messages in thread
From: Sandipan Das @ 2026-10-15 11:46 UTC (permalink / raw)
  To: linux-perf-users, linux-kernel, bpf
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark, Thomas Gleixner,
	Borislav Petkov, Dave Hansen, x86, H . Peter Anvin, Dapeng Mi,
	Andrii Nakryiko, Song Liu, Ravi Bangoria, Ananth Narayan,
	Sandipan Das

Sashiko reported that the vendor-specific branch snapshot callbacks
used by bpf_get_branch_snapshot() copy up to x86_pmu.lbr_nr entries
from cpuc->lbr_entries[], regardless of how many valid entries the
last LBR read produced. This makes BPF programs read stale or invalid
branch records. Patch 1 addresses AMD and patch 2 addresses Intel.

Previous version(s) can be found at
v1: https://lore.kernel.org/all/20261007181002.4005262-1-sandipan.das@amd.com/

Changes in v2:
  * Include a fix for Intel LBR.
  * Add Tested-by from Dapeng to the Intel fix.
    https://lore.kernel.org/all/eb6c7bda-0dcf-4fac-ad69-9f5b24abc21b@linux.intel.com/

Sandipan Das (2):
  perf/x86/amd/lbr: Limit branch snapshots to valid entries
  perf/x86/intel/lbr: Limit branch snapshots to valid entries

 arch/x86/events/amd/core.c   | 2 +-
 arch/x86/events/amd/lbr.c    | 4 +++-
 arch/x86/events/intel/core.c | 2 +-
 arch/x86/events/intel/lbr.c  | 4 +++-
 4 files changed, 8 insertions(+), 4 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] perf/x86/amd/lbr: Limit branch snapshots to valid entries
  2026-10-15 11:46 [PATCH v2 0/2] perf/x86: Limit branch snapshots to valid entries Sandipan Das
@ 2026-10-15 11:46 ` Sandipan Das
  2026-10-16  0:14   ` Mi, Dapeng
  2026-10-15 11:46 ` [PATCH v2 2/2] perf/x86/intel/lbr: " Sandipan Das
  1 sibling, 1 reply; 6+ messages in thread
From: Sandipan Das @ 2026-10-15 11:46 UTC (permalink / raw)
  To: linux-perf-users, linux-kernel, bpf
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark, Thomas Gleixner,
	Borislav Petkov, Dave Hansen, x86, H . Peter Anvin, Dapeng Mi,
	Andrii Nakryiko, Song Liu, Ravi Bangoria, Ananth Narayan,
	Sandipan Das

The branch snapshot callback used by bpf_get_branch_snapshot() copies
up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
last LBR read kept fewer branches. The entries past lbr_stack.nr are
invalid, but they end up reaching BPF programs.

Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
amd_pmu_lbr_read() skips reading the registers, so that a stale count
is never reused.

Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
Signed-off-by: Sandipan Das <sandipan.das@amd.com>
---
 arch/x86/events/amd/core.c | 2 +-
 arch/x86/events/amd/lbr.c  | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 49b6b8fce566..bf631f60757c 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -944,7 +944,7 @@ static int amd_pmu_v2_snapshot_branch_stack(struct perf_branch_entry *entries, u
 	cpuc = this_cpu_ptr(&cpu_hw_events);
 
 	amd_pmu_lbr_read();
-	cnt = min(cnt, x86_pmu.lbr_nr);
+	cnt = min(cnt, cpuc->lbr_stack.nr);
 	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
 
 	amd_pmu_v2_enable_all(0);
diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
index 1e1b2f08cb51..a00d3850c4a8 100644
--- a/arch/x86/events/amd/lbr.c
+++ b/arch/x86/events/amd/lbr.c
@@ -165,8 +165,10 @@ void amd_pmu_lbr_read(void)
 	struct branch_entry entry;
 	int out = 0, idx, i;
 
-	if (!cpuc->lbr_users)
+	if (!cpuc->lbr_users) {
+		cpuc->lbr_stack.nr = 0;
 		return;
+	}
 
 	for (i = 0; i < x86_pmu.lbr_nr; i++) {
 		entry.from.full	= amd_pmu_lbr_get_from(i);
-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] perf/x86/intel/lbr: Limit branch snapshots to valid entries
  2026-10-15 11:46 [PATCH v2 0/2] perf/x86: Limit branch snapshots to valid entries Sandipan Das
  2026-10-15 11:46 ` [PATCH v2 1/2] perf/x86/amd/lbr: " Sandipan Das
@ 2026-10-15 11:46 ` Sandipan Das
  2026-10-16  0:14   ` Mi, Dapeng
  2026-10-16  0:21   ` bot+bpf-ci
  1 sibling, 2 replies; 6+ messages in thread
From: Sandipan Das @ 2026-10-15 11:46 UTC (permalink / raw)
  To: linux-perf-users, linux-kernel, bpf
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark, Thomas Gleixner,
	Borislav Petkov, Dave Hansen, x86, H . Peter Anvin, Dapeng Mi,
	Andrii Nakryiko, Song Liu, Ravi Bangoria, Ananth Narayan,
	Sandipan Das

The branch snapshot callback used by bpf_get_branch_snapshot() always
copies x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
last LBR read kept fewer branches. The entries past lbr_stack.nr hold
invalid branches which end up reaching BPF programs.

Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
intel_pmu_lbr_read() skips reading the registers so that a stale count
is never reused.

Fixes: c22ac2a3d4bd ("perf: Enable branch record for software events")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
Signed-off-by: Sandipan Das <sandipan.das@amd.com>
Tested-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 arch/x86/events/intel/core.c | 2 +-
 arch/x86/events/intel/lbr.c  | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 0a34d674df59..5dfb9e9156e8 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2822,7 +2822,7 @@ __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 
 	intel_pmu_lbr_read();
-	cnt = min_t(unsigned int, cnt, x86_pmu.lbr_nr);
+	cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
 
 	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
 	intel_pmu_enable_all(0);
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 0b446d8912e4..6b070490f461 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1026,8 +1026,10 @@ void intel_pmu_lbr_read(void)
 	 * but this simple approach seems to work for now.
 	 */
 	if (!cpuc->lbr_users || vlbr_exclude_host() ||
-	    cpuc->lbr_users == cpuc->lbr_pebs_users)
+	    cpuc->lbr_users == cpuc->lbr_pebs_users) {
+		cpuc->lbr_stack.nr = 0;
 		return;
+	}
 
 	x86_pmu.lbr_read(cpuc);
 
-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] perf/x86/amd/lbr: Limit branch snapshots to valid entries
  2026-10-15 11:46 ` [PATCH v2 1/2] perf/x86/amd/lbr: " Sandipan Das
@ 2026-10-16  0:14   ` Mi, Dapeng
  0 siblings, 0 replies; 6+ messages in thread
From: Mi, Dapeng @ 2026-10-16  0:14 UTC (permalink / raw)
  To: Sandipan Das, linux-perf-users, linux-kernel, bpf
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark, Thomas Gleixner,
	Borislav Petkov, Dave Hansen, x86, H . Peter Anvin,
	Andrii Nakryiko, Song Liu, Ravi Bangoria, Ananth Narayan

LGTM. Thanks.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>


On 10/15/2026 7:46 PM, Sandipan Das wrote:
> The branch snapshot callback used by bpf_get_branch_snapshot() copies
> up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
> last LBR read kept fewer branches. The entries past lbr_stack.nr are
> invalid, but they end up reaching BPF programs.
>
> Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
> amd_pmu_lbr_read() skips reading the registers, so that a stale count
> is never reused.
>
> Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> ---
>  arch/x86/events/amd/core.c | 2 +-
>  arch/x86/events/amd/lbr.c  | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 49b6b8fce566..bf631f60757c 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -944,7 +944,7 @@ static int amd_pmu_v2_snapshot_branch_stack(struct perf_branch_entry *entries, u
>  	cpuc = this_cpu_ptr(&cpu_hw_events);
>  
>  	amd_pmu_lbr_read();
> -	cnt = min(cnt, x86_pmu.lbr_nr);
> +	cnt = min(cnt, cpuc->lbr_stack.nr);
>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
>  
>  	amd_pmu_v2_enable_all(0);
> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
> index 1e1b2f08cb51..a00d3850c4a8 100644
> --- a/arch/x86/events/amd/lbr.c
> +++ b/arch/x86/events/amd/lbr.c
> @@ -165,8 +165,10 @@ void amd_pmu_lbr_read(void)
>  	struct branch_entry entry;
>  	int out = 0, idx, i;
>  
> -	if (!cpuc->lbr_users)
> +	if (!cpuc->lbr_users) {
> +		cpuc->lbr_stack.nr = 0;
>  		return;
> +	}
>  
>  	for (i = 0; i < x86_pmu.lbr_nr; i++) {
>  		entry.from.full	= amd_pmu_lbr_get_from(i);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] perf/x86/intel/lbr: Limit branch snapshots to valid entries
  2026-10-15 11:46 ` [PATCH v2 2/2] perf/x86/intel/lbr: " Sandipan Das
@ 2026-10-16  0:14   ` Mi, Dapeng
  2026-10-16  0:21   ` bot+bpf-ci
  1 sibling, 0 replies; 6+ messages in thread
From: Mi, Dapeng @ 2026-10-16  0:14 UTC (permalink / raw)
  To: Sandipan Das, linux-perf-users, linux-kernel, bpf
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, James Clark, Thomas Gleixner,
	Borislav Petkov, Dave Hansen, x86, H . Peter Anvin,
	Andrii Nakryiko, Song Liu, Ravi Bangoria, Ananth Narayan

LGTM. Thanks.

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>


On 10/15/2026 7:46 PM, Sandipan Das wrote:
> The branch snapshot callback used by bpf_get_branch_snapshot() always
> copies x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
> last LBR read kept fewer branches. The entries past lbr_stack.nr hold
> invalid branches which end up reaching BPF programs.
>
> Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
> intel_pmu_lbr_read() skips reading the registers so that a stale count
> is never reused.
>
> Fixes: c22ac2a3d4bd ("perf: Enable branch record for software events")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> Tested-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---
>  arch/x86/events/intel/core.c | 2 +-
>  arch/x86/events/intel/lbr.c  | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 0a34d674df59..5dfb9e9156e8 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -2822,7 +2822,7 @@ __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>  
>  	intel_pmu_lbr_read();
> -	cnt = min_t(unsigned int, cnt, x86_pmu.lbr_nr);
> +	cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
>  
>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
>  	intel_pmu_enable_all(0);
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 0b446d8912e4..6b070490f461 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1026,8 +1026,10 @@ void intel_pmu_lbr_read(void)
>  	 * but this simple approach seems to work for now.
>  	 */
>  	if (!cpuc->lbr_users || vlbr_exclude_host() ||
> -	    cpuc->lbr_users == cpuc->lbr_pebs_users)
> +	    cpuc->lbr_users == cpuc->lbr_pebs_users) {
> +		cpuc->lbr_stack.nr = 0;
>  		return;
> +	}
>  
>  	x86_pmu.lbr_read(cpuc);
>  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] perf/x86/intel/lbr: Limit branch snapshots to valid entries
  2026-10-15 11:46 ` [PATCH v2 2/2] perf/x86/intel/lbr: " Sandipan Das
  2026-10-16  0:14   ` Mi, Dapeng
@ 2026-10-16  0:21   ` bot+bpf-ci
  1 sibling, 0 replies; 6+ messages in thread
From: bot+bpf-ci @ 2026-10-16  0:21 UTC (permalink / raw)
  To: sandipan.das, linux-perf-users, linux-kernel, bpf
  Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
	jolsa, irogers, adrian.hunter, james.clark, tglx, bp,
	dave.hansen, x86, hpa, dapeng1.mi, andrii, song, ravi.bangoria,
	ananth.narayan, sandipan.das, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 3238 bytes --]

> perf/x86/intel/lbr: Limit branch snapshots to valid entries

TL;DR: Resetting cpuc->lbr_stack.nr in intel_pmu_lbr_read() may clobber an
in-flight adaptive PEBS sample if a BPF program calls
bpf_get_branch_snapshot(), possibly yielding a PERF_RECORD_SAMPLE without
branch records and with leftover bytes.

> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 22e2a06d5786..2fe858d99979 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c

[ ... ]

> @@ -1026,8 +1026,10 @@ void intel_pmu_lbr_read(void)
>  	 * but this simple approach seems to work for now.
>  	 */
>  	if (!cpuc->lbr_users || vlbr_exclude_host() ||
> -	    cpuc->lbr_users == cpuc->lbr_pebs_users)
> +	    cpuc->lbr_users == cpuc->lbr_pebs_users) {
> +		cpuc->lbr_stack.nr = 0;
                 ^^^^
Can this clobber the branch stack of a PEBS sample that is still being
built?

Before this patch the early return had no side effects.  Now it writes to
cpuc->lbr_stack, which is shared with the adaptive PEBS path.

If every LBR user on the CPU is an adaptive PEBS event, then
cpuc->lbr_users == cpuc->lbr_pebs_users, and the PEBS drain points the
sample at the per-CPU stack:

setup_pebs_adaptive_sample_data()
    intel_pmu_store_pebs_lbrs(lbr);         /* cpuc->lbr_stack.nr = N */
    intel_pmu_lbr_save_brstack(data, cpuc, event);
        perf_sample_save_brstack(data, event, &cpuc->lbr_stack, ...);
            data->br_stack = brs;           /* points at cpuc->lbr_stack */
            data->dyn_size += ... + brs->nr * sizeof(struct perf_branch_entry);

__intel_pmu_pebs_last_event() then calls perf_event_overflow(), and
__perf_event_overflow() runs bpf_overflow_handler() before the sample is
written.  A perf event BPF program attached to that event can call
bpf_get_branch_snapshot(), which is in bpf_base_func_proto:

intel_pmu_snapshot_branch_stack()
    __intel_pmu_snapshot_branch_stack()
        intel_pmu_lbr_read()
            /* lbr_users == lbr_pebs_users, early return taken */
            cpuc->lbr_stack.nr = 0;

When the program returns, data->br_stack still points at cpuc->lbr_stack,
but nr is now 0.  A later bpf_read_branch_records() in the same program
reads ctx->data->br_stack->nr and returns 0 bytes.

perf_output_sample() then writes nr = 0 and no entries, while header.size was
already computed by perf_prepare_header() from data->dyn_size, which counted
N entries.  Doesn't that leave the PERF_RECORD_SAMPLE without its branch
records and followed by N * sizeof(struct perf_branch_entry) bytes of
leftover ring buffer content?

Before the patch, the early return left cpuc->lbr_stack untouched and this
sample came out intact.

Would it be better to leave cpuc->lbr_stack alone in the snapshot path, for
example by checking the skip condition in __intel_pmu_snapshot_branch_stack()
and returning 0 there, instead of resetting the shared nr inside
intel_pmu_lbr_read() for every caller?

>  		return;
> +	}
>
>  	x86_pmu.lbr_read(cpuc);


---
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/38827450009

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-16  0:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-15 11:46 [PATCH v2 0/2] perf/x86: Limit branch snapshots to valid entries Sandipan Das
2026-10-15 11:46 ` [PATCH v2 1/2] perf/x86/amd/lbr: " Sandipan Das
2026-10-16  0:14   ` Mi, Dapeng
2026-10-15 11:46 ` [PATCH v2 2/2] perf/x86/intel/lbr: " Sandipan Das
2026-10-16  0:14   ` Mi, Dapeng
2026-10-16  0:21   ` 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

all inboxes | Powered by JetHome®