[Patch v3 0/4] perf/x86: Limit branch snapshots to valid entries

[Patch v3 0/4] perf/x86: Limit branch snapshots to valid entries

[Dapeng Mi]

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.

Changes in v3:
- Patch 01: A new patch to ensure x86_pmu.lbr_nr not to exclude
  MAX_LBR_ENTRIES (Internal Sashiko).
- Patch 03: Clean cached LBR stack when reseting hardware LBR registers,
  which prevents stale LBR stack leakage.
- Patch 04: Don't reset cached LBR stack if all LBR events are PEBS
  events. This ensures PEBS captured LBR stack isn't cleared (Sashiko).

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/

History:
v2: https://lore.kernel.org/all/cover.1792064123.git.sandipan.das@amd.com/
v1: https://lore.kernel.org/all/20261007181002.4005262-1-sandipan.das@amd.com/

Dapeng Mi (3):
  perf/x86: Clamp x86_pmu.lbr_nr to MAX_LBR_ENTRIES
  perf/x86/intel/lbr: Reset the cached LBR stack in
    intel_pmu_lbr_reset()
  perf/x86/intel/lbr: Limit branch snapshots to valid entries

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

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


base-commit: f5a78ecccd98a04c47cea1cd63c2a2fdce8a4b48
-- 
2.34.1

[Patch v3 1/4] perf/x86: Clamp x86_pmu.lbr_nr to MAX_LBR_ENTRIES

[Dapeng Mi]

Ensure x86_pmu.lbr_nr never exceeds MAX_LBR_ENTRIES to avoid
out-of-bounds access when handling the cached LBR stack.

Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 arch/x86/events/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 4df792b139e0..82f6b66db7e2 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2645,6 +2645,8 @@ static int __init init_hw_perf_events(void)
 	if (!x86_pmu.config_mask)
 		x86_pmu.config_mask = X86_RAW_EVENT_MASK;
 
+	x86_pmu.lbr_nr = min(x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
+
 	perf_events_lapic_init();
 	register_nmi_handler(NMI_LOCAL, perf_event_nmi_handler, 0, "PMI");
 
-- 
2.34.1

[Patch v3 2/4] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Dapeng Mi]

From: Sandipan Das <sandipan.das@amd.com>

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>
Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.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 a55646fcb846..6c93b8042c73 100644
--- a/arch/x86/events/amd/lbr.c
+++ b/arch/x86/events/amd/lbr.c
@@ -168,8 +168,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.34.1

[Patch v3 3/4] perf/x86/intel/lbr: Reset the cached LBR stack in intel_pmu_lbr_reset()

[Dapeng Mi]

Reset the cached LBR stack in addition to the hardware LBR register file
when intel_pmu_lbr_reset() is called. This prevents stale LBR data from
being reused after a context switch or similar transition.

Also adjust intel_pmu_arch_lbr_read_xsave() to update the cached LBR
stack only when new hardware LBR data is present, avoiding overwrite of
stale data with previously captured values.

Fixes: c085fb877467 ("perf/x86/intel/lbr: Support XSAVES for arch LBR read")
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 arch/x86/events/intel/lbr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 0b446d8912e4..249db12e927e 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -193,6 +193,8 @@ void intel_pmu_lbr_reset(void)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 
+	cpuc->lbr_stack.nr = 0;
+
 	if (!x86_pmu.lbr_nr)
 		return;
 
@@ -1012,7 +1014,8 @@ static void intel_pmu_arch_lbr_read_xsave(struct cpu_hw_events *cpuc)
 	}
 	xsaves(&xsave->xsave, XFEATURE_MASK_LBR);
 
-	intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
+	if (xsave->header.xfeatures & XFEATURE_MASK_LBR)
+		intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
 }
 
 void intel_pmu_lbr_read(void)
-- 
2.34.1

[Patch v3 4/4] perf/x86/intel/lbr: Limit branch snapshots to valid entries

[Dapeng Mi]

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
no active LBR events or LBR hardware is owned by guest. So the stale or
guest LBR stack data can't be leaked.

When all active LBR users are PEBS events, the LBR stack is already
updated from the PEBS records, so there is no need to read the LBR
hardware again.

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/
Co-developed-by: Sandipan Das <sandipan.das@amd.com>
Signed-off-by: Sandipan Das <sandipan.das@amd.com>
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 arch/x86/events/intel/core.c | 2 +-
 arch/x86/events/intel/lbr.c  | 9 +++++++--
 2 files changed, 8 insertions(+), 3 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 249db12e927e..cc0300aba7de 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1022,14 +1022,19 @@ void intel_pmu_lbr_read(void)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 
+	if (!cpuc->lbr_users || vlbr_exclude_host()) {
+		/* Reset lbr_stack.nr to prevent stale data leakage. */
+		cpuc->lbr_stack.nr = 0;
+		return;
+	}
+
 	/*
 	 * Don't read when all LBRs users are using adaptive PEBS.
 	 *
 	 * This could be smarter and actually check the event,
 	 * but this simple approach seems to work for now.
 	 */
-	if (!cpuc->lbr_users || vlbr_exclude_host() ||
-	    cpuc->lbr_users == cpuc->lbr_pebs_users)
+	if (cpuc->lbr_users == cpuc->lbr_pebs_users)
 		return;
 
 	x86_pmu.lbr_read(cpuc);
-- 
2.34.1

Re: [Patch v3 2/4] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[sashiko-bot]

> From: Sandipan Das <sandipan.das@amd.com>
> 
> 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>
> Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019070836.2972403-1-dapeng1.mi@linux.intel.com?part=2

Re: [Patch v3 1/4] perf/x86: Clamp x86_pmu.lbr_nr to MAX_LBR_ENTRIES

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf/x86: late x86_pmu.lbr_nr clamp in init_hw_perf_events() risks #GP
--

commit 7f534b76717bd0a7a33fa3404815780334bcf485
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf/x86: Clamp x86_pmu.lbr_nr to MAX_LBR_ENTRIES

This commit clamps x86_pmu.lbr_nr to MAX_LBR_ENTRIES in
init_hw_perf_events() to prevent out-of-bounds accesses to the cached
LBR stack.

> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 4df792b139e05..82f6b66db7e2f 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -2645,6 +2645,8 @@ static int __init init_hw_perf_events(void)
>  	if (!x86_pmu.config_mask)
>  		x86_pmu.config_mask = X86_RAW_EVENT_MASK;
>  
> +	x86_pmu.lbr_nr = min(x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
> +
>  	perf_events_lapic_init();
>  	register_nmi_handler(NMI_LOCAL, perf_event_nmi_handler, 0, "PMI");

[Severity: High]
Does clamping x86_pmu.lbr_nr after intel_pmu_init() and amd_pmu_init()
return risk an unhandled #GP in intel_pmu_arch_lbr_reset() and leave
the vendor PMU initialization inconsistent with the clamped depth?

For Intel Architectural LBR, supported depths are enumerated as a bitmask
in eax.split.lbr_depth_mask (where each bit indicates support for a
multiple of 8 up to 64). During init_hw_perf_events() -> intel_pmu_init()
-> intel_pmu_arch_lbr_init(), the code selects the highest supported bit
and validates that unclamped depth against MSR_ARCH_LBR_DEPTH:

arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_init() {
	...
	lbr_nr = fls(eax.split.lbr_depth_mask) * 8;
	if (!lbr_nr)
		goto clear_arch_lbr;

	/* Apply the max depth of Arch LBR */
	if (wrmsrq_safe(MSR_ARCH_LBR_DEPTH, lbr_nr))
		goto clear_arch_lbr;
	...
}

If a CPU or hypervisor advertises a depth greater than 32 in
lbr_depth_mask without also setting the bit for depth 32, clamping
x86_pmu.lbr_nr to MAX_LBR_ENTRIES (32) here leaves x86_pmu.lbr_nr set to
an unsupported depth that was never tested with wrmsrq_safe(). When
cpuhp_setup_state() subsequently runs x86_pmu_starting_cpu() ->
intel_pmu_cpu_starting() -> intel_pmu_lbr_reset() ->
intel_pmu_arch_lbr_reset():

arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_reset() {
	/* Write to ARCH_LBR_DEPTH MSR, all LBR entries are reset to 0 */
	wrmsrq(MSR_ARCH_LBR_DEPTH, x86_pmu.lbr_nr);
}

won't this unchecked wrmsrq() of 32 to MSR_ARCH_LBR_DEPTH raise a fatal
#GP at boot?

Additionally, because this clamp runs after vendor initialization has
completed:

1. intel_pmu_arch_lbr_init() has already validated the XSAVE size via
   is_arch_lbr_xsave_available() and allocated pmu->task_ctx_cache using
   the unclamped lbr_nr.

2. intel_pmu_init() has already probed MSRs up to the unclamped
   x86_pmu.lbr_nr and logged the unclamped depth:

arch/x86/events/intel/core.c:intel_pmu_init() {
	...
	for (i = 0; i < x86_pmu.lbr_nr; i++) {
		if (!(check_msr(x86_pmu.lbr_from + i, 0xffffUL) &&
		      check_msr(x86_pmu.lbr_to + i, 0xffffUL)))
			x86_pmu.lbr_nr = 0;
	}

	if (x86_pmu.lbr_nr) {
		intel_pmu_lbr_init();

		pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
	...
}

3. amd_pmu_lbr_init() has likewise already logged the unclamped depth:

arch/x86/events/amd/lbr.c:amd_pmu_lbr_init() {
	...
	x86_pmu.lbr_nr = ebx.split.lbr_v2_stack_sz;

	pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
	...
}

Should the MAX_LBR_ENTRIES limit be enforced at the point where
x86_pmu.lbr_nr (and lbr_depth_mask in intel_pmu_arch_lbr_init()) is
determined inside the vendor initialization paths instead?

>  
 완벽

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019070836.2972403-1-dapeng1.mi@linux.intel.com?part=1

Re: [Patch v3 3/4] perf/x86/intel/lbr: Reset the cached LBR stack in intel_pmu_lbr_reset()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf/x86/intel/lbr: stale lbr_stack.nr in intel_pmu_arch_lbr_read_xsave()
--

commit 93eb502f5c2972255761d542db115fed920f52bd
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf/x86/intel/lbr: Reset the cached LBR stack in intel_pmu_lbr_reset()

This commit resets cpuc->lbr_stack.nr in intel_pmu_lbr_reset() and skips
calling intel_pmu_store_lbr() in intel_pmu_arch_lbr_read_xsave() when
XFEATURE_MASK_LBR is not set in xsave->header.xfeatures.

> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 0b446d8912e4e..249db12e927ed 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -193,6 +193,8 @@ void intel_pmu_lbr_reset(void)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>  
> +	cpuc->lbr_stack.nr = 0;
> +
>  	if (!x86_pmu.lbr_nr)
>  		return;
>  
> @@ -1012,7 +1014,8 @@ static void intel_pmu_arch_lbr_read_xsave(struct cpu_hw_events *cpuc)
>  	}
>  	xsaves(&xsave->xsave, XFEATURE_MASK_LBR);
>  
> -	intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
> +	if (xsave->header.xfeatures & XFEATURE_MASK_LBR)
> +		intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
>  }
>  
>  void intel_pmu_lbr_read(void)

[Severity: High]
When xsaves() sees the hardware LBR state in its INIT configuration, the
XSAVE init optimization clears XFEATURE_MASK_LBR in
xsave->header.xfeatures without writing to xsave->lbr.entries.

If intel_pmu_store_lbr() is skipped here without resetting
cpuc->lbr_stack.nr = 0, can cpuc->lbr_stack.nr and cpuc->lbr_entries[]
retain stale branch entries whenever the hardware LBR state enters the
INIT configuration without going through intel_pmu_lbr_reset()?

For example, during a context switch in LBR call-stack mode,
__intel_pmu_lbr_restore() restores the incoming task's LBR state without
calling intel_pmu_lbr_reset():

arch/x86/events/intel/lbr.c:__intel_pmu_lbr_restore() {
	if (!has_lbr_callstack_users(ctx) ||
	    task_context_opt(ctx)->lbr_stack_state == LBR_NONE) {
		intel_pmu_lbr_reset();
		return;
	}
	...
	x86_pmu.lbr_restore(ctx);

	task_context_opt(ctx)->lbr_stack_state = LBR_NONE;
}

If the previous task populated cpuc->lbr_stack.nr > 0 and the incoming
task had 0 branches when scheduled out (so xrstors() restores the hardware
LBR state to the INIT state), a PMI or branch snapshot in the new task
before new branches are recorded will see !(xsave->header.xfeatures &
XFEATURE_MASK_LBR) and leave the previous task's cpuc->lbr_stack.nr and
cpuc->lbr_entries[] in place.

The same situation can also occur within a single task when:
- RET instructions in call-stack mode pop all hardware LBR entries back to
  the INIT state after an earlier PMI populated cpuc->lbr_stack.nr,
- a deep C-state resets the hardware Arch LBR registers to the INIT state
  (x86_pmu.lbr_deep_c_reset) without calling intel_pmu_lbr_reset(), or
- intel_pmu_store_pebs_lbrs() populates cpuc->lbr_stack.nr from a PEBS
  record while the hardware LBR registers are in the INIT state.

In addition to leaking or repeating stale branch entries,
intel_pmu_lbr_read() then runs intel_pmu_lbr_filter(cpuc) a second time on
the stale cpuc->lbr_entries[], where e->type already holds a converted
PERF_BR_* value rather than a raw ARCH_LBR_BR_TYPE_* value, indexing
arch_lbr_br_type_map[] with the wrong value.

Should intel_pmu_arch_lbr_read_xsave() clear cpuc->lbr_stack.nr = 0 when
!(xsave->header.xfeatures & XFEATURE_MASK_LBR)?

	if (xsave->header.xfeatures & XFEATURE_MASK_LBR)
		intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
	else
		cpuc->lbr_stack.nr = 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019070836.2972403-1-dapeng1.mi@linux.intel.com?part=3

Re: [Patch v3 4/4] perf/x86/intel/lbr: Limit branch snapshots to valid entries

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] perf/x86/intel/lbr: stale LBR data or -ENOENT in intel_pmu_lbr_read() for PEBS
- [Medium] perf/x86/intel/lbr: unreset lbr_stack.nr in intel_pmu_arch_lbr_read_xsave()
- [Medium] perf/x86: late lbr_nr clamping in init_hw_perf_events() after arch LBR init
--

commit da27a6ac96e6d944e33e171c276d0b044be96def
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

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

This patch limits __intel_pmu_snapshot_branch_stack() to copy up to
cpuc->lbr_stack.nr entries rather than x86_pmu.lbr_nr, and resets
cpuc->lbr_stack.nr to 0 in intel_pmu_lbr_read() when there are no active
LBR users or LBR is excluded on the host.

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 0a34d674df590..5dfb9e9156e88 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 249db12e927ed..cc0300aba7de0 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1022,14 +1022,19 @@ void intel_pmu_lbr_read(void)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>  
> +	if (!cpuc->lbr_users || vlbr_exclude_host()) {
> +		/* Reset lbr_stack.nr to prevent stale data leakage. */
> +		cpuc->lbr_stack.nr = 0;
> +		return;
> +	}
> +
>  	/*
>  	 * Don't read when all LBRs users are using adaptive PEBS.
>  	 *
>  	 * This could be smarter and actually check the event,
>  	 * but this simple approach seems to work for now.
>  	 */
> -	if (!cpuc->lbr_users || vlbr_exclude_host() ||
> -	    cpuc->lbr_users == cpuc->lbr_pebs_users)
> +	if (cpuc->lbr_users == cpuc->lbr_pebs_users)
>  		return;

[Severity: High]
Does skipping x86_pmu.lbr_read(cpuc) here when cpuc->lbr_users ==
cpuc->lbr_pebs_users cause __intel_pmu_snapshot_branch_stack() to return
0 entries or copy stale cpuc->lbr_entries[] to BPF programs?

The commit message notes that when all active LBR users are PEBS events,
the LBR stack is already updated from the PEBS records. However, PEBS LBR
records are only written into cpuc->lbr_entries[] and cpuc->lbr_stack.nr
during counter overflow PMI draining:

  handle_pmi_common()
    -> x86_pmu_drain_pebs()
      -> setup_pebs_adaptive_sample_data() / setup_arch_pebs_sample_data()
        -> intel_pmu_store_pebs_lbrs()

In contrast, bpf_get_branch_snapshot() invokes
intel_pmu_snapshot_branch_stack() or intel_pmu_snapshot_arch_branch_stack()
synchronously from a software or tracing event outside of any PEBS drain.
Those functions freeze the hardware LBRs and call
__intel_pmu_snapshot_branch_stack() -> intel_pmu_lbr_read() to read the
current hardware LBR registers.

When all active LBR events on the CPU are adaptive PEBS events, returning
early in intel_pmu_lbr_read() without reading the hardware LBRs or
updating cpuc->lbr_stack.nr leads to several issues:

1. If no PEBS PMI has occurred since the last intel_pmu_lbr_reset(),
   cpuc->lbr_stack.nr is still 0, so __intel_pmu_snapshot_branch_stack()
   copies 0 entries and bpf_get_branch_snapshot() fails with -ENOENT even
   though the LBR hardware has been actively recording branches.

2. If an earlier PEBS PMI occurred, cpuc->lbr_entries[] and
   cpuc->lbr_stack.nr hold the historical branch stack from that old PEBS
   overflow sample rather than the live branches frozen at the snapshot
   point.

3. Stale or cross-task entries can also remain in cpuc->lbr_entries[] with
   cpuc->lbr_stack.nr > 0 across transitions that do not call
   intel_pmu_lbr_reset():

   arch/x86/events/intel/lbr.c:intel_pmu_lbr_add() {
       if (x86_pmu.intel_cap.pebs_baseline && event->attr.precise_ip > 0)
           cpuc->lbr_pebs_users++;
       perf_sched_cb_inc(event->pmu);
       if (!cpuc->lbr_users++ && !event->total_time_running)
           intel_pmu_lbr_reset();
   }

   Scheduling in a previously active PEBS LBR event where
   event->total_time_running > 0 (or removing a non-PEBS LBR event via
   intel_pmu_lbr_del() while a PEBS LBR event remains active) does not
   reset cpuc->lbr_stack.nr. Similarly, on a context switch with
   PERF_SAMPLE_BRANCH_CALL_STACK:

   arch/x86/events/intel/lbr.c:__intel_pmu_lbr_restore() {
       ...
       x86_pmu.lbr_restore(ctx);

       task_context_opt(ctx)->lbr_stack_state = LBR_NONE;
   }

   __intel_pmu_lbr_restore() updates the hardware LBR MSRs for the
   incoming task without resetting cpuc->lbr_stack.nr, leaving the
   previous task's cached cpuc->lbr_entries[] to be copied by
   __intel_pmu_snapshot_branch_stack().

>  
>  	x86_pmu.lbr_read(cpuc);

[Severity: Medium]
Can x86_pmu.lbr_read(cpuc) leave a stale non-zero cpuc->lbr_stack.nr when
intel_pmu_arch_lbr_read_xsave() sees the hardware Arch LBR state in its
XSAVE INIT configuration?

In the preceding patch ("perf/x86/intel/lbr: Reset the cached LBR stack in
intel_pmu_lbr_reset()"), intel_pmu_arch_lbr_read_xsave() was updated to:

arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_read_xsave() {
	...
	xsaves(&xsave->xsave, XFEATURE_MASK_LBR);

	if (xsave->header.xfeatures & XFEATURE_MASK_LBR)
		intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
}

When 0 branches have been recorded in hardware, the Arch LBR state is in
its XSAVE INIT configuration, so xsaves() clears XFEATURE_MASK_LBR in
xsave->header.xfeatures. Unlike the MSR fallback path
intel_pmu_store_lbr(cpuc, NULL) which reads 0 from MSR_ARCH_LBR_FROM_0 and
sets cpuc->lbr_stack.nr = 0, skipping intel_pmu_store_lbr() here without
an else branch leaves cpuc->lbr_stack.nr at its previous value.

If cpuc->lbr_stack.nr was non-zero from an earlier read and the hardware
Arch LBR state subsequently returns to the INIT state without
intel_pmu_lbr_reset() being called (for example, when
__intel_pmu_lbr_restore() restores a task's call-stack LBR context via
intel_pmu_arch_lbr_xrstors() whose saved xfeatures has XFEATURE_MASK_LBR
clear, when RET instructions pop all prior CALL entries in call-stack
mode, when a CPU wakes from a deep C-state that resets Arch LBR registers,
or when an LBR event with event->total_time_running > 0 is scheduled back
in), __intel_pmu_snapshot_branch_stack() will see the old
cpuc->lbr_stack.nr and copy stale cpuc->lbr_entries[] into entries.

Should intel_pmu_arch_lbr_read_xsave() set cpuc->lbr_stack.nr = 0 when
!(xsave->header.xfeatures & XFEATURE_MASK_LBR)?


[Severity: Medium]
Also in the earlier patch in this series ("perf/x86: Clamp x86_pmu.lbr_nr
to MAX_LBR_ENTRIES"), init_hw_perf_events() clamps x86_pmu.lbr_nr after
vendor PMU initialization has already run:

arch/x86/events/core.c:init_hw_perf_events() {
	...
	err = x86_pmu.init();
	...
	x86_pmu.lbr_nr = min(x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
}

During x86_pmu.init() -> intel_pmu_init() -> intel_pmu_arch_lbr_init(),
the Arch LBR depth is selected from the highest bit in
eax.split.lbr_depth_mask (up to 64), tested against MSR_ARCH_LBR_DEPTH,
and validated against xfeature_size(XFEATURE_LBR):

arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_init() {
	...
	lbr_nr = fls(eax.split.lbr_depth_mask) * 8;
	if (!lbr_nr)
		goto clear_arch_lbr;

	/* Apply the max depth of Arch LBR */
	if (wrmsrq_safe(MSR_ARCH_LBR_DEPTH, lbr_nr))
		goto clear_arch_lbr;

	x86_pmu.lbr_nr = lbr_nr;

	if (has_arch_lbr_xsave() &&
	    xfeature_size(XFEATURE_LBR) != get_lbr_state_size()) {
		WARN_ON(1);
		goto clear_arch_lbr;
	}
}

If hardware enumerates a maximum depth greater than MAX_LBR_ENTRIES (32)
in eax.split.lbr_depth_mask, clamping x86_pmu.lbr_nr to 32 afterwards in
init_hw_perf_events() means intel_pmu_arch_lbr_reset() will later execute
an unchecked wrmsrq(MSR_ARCH_LBR_DEPTH, x86_pmu.lbr_nr) with depth 32:

arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_reset() {
	/* Write to ARCH_LBR_DEPTH MSR, all LBR entries are reset to 0 */
	wrmsrq(MSR_ARCH_LBR_DEPTH, x86_pmu.lbr_nr);
}

If depth 32 (bit 3 in eax.split.lbr_depth_mask) is not supported by the
processor or VM, won't that wrmsrq(MSR_ARCH_LBR_DEPTH, 32) trigger an
unhandled #GP fault during CPU startup or LBR reset, in addition to
get_lbr_state_size() and the boot logs having used the unclamped
x86_pmu.lbr_nr?

Should intel_pmu_arch_lbr_init() mask or bound the selected depth to
MAX_LBR_ENTRIES before validating MSR_ARCH_LBR_DEPTH and
get_lbr_state_size()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019070836.2972403-1-dapeng1.mi@linux.intel.com?part=4

Re: [Patch v3 1/4] perf/x86: Clamp x86_pmu.lbr_nr to MAX_LBR_ENTRIES

[Mi, Dapeng]

On 10/19/2026 3:28 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] perf/x86: late x86_pmu.lbr_nr clamp in init_hw_perf_events() risks #GP
> --
>
> commit 7f534b76717bd0a7a33fa3404815780334bcf485
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86: Clamp x86_pmu.lbr_nr to MAX_LBR_ENTRIES
>
> This commit clamps x86_pmu.lbr_nr to MAX_LBR_ENTRIES in
> init_hw_perf_events() to prevent out-of-bounds accesses to the cached
> LBR stack.
>
>> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
>> index 4df792b139e05..82f6b66db7e2f 100644
>> --- a/arch/x86/events/core.c
>> +++ b/arch/x86/events/core.c
>> @@ -2645,6 +2645,8 @@ static int __init init_hw_perf_events(void)
>>  	if (!x86_pmu.config_mask)
>>  		x86_pmu.config_mask = X86_RAW_EVENT_MASK;
>>  
>> +	x86_pmu.lbr_nr = min(x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
>> +
>>  	perf_events_lapic_init();
>>  	register_nmi_handler(NMI_LOCAL, perf_event_nmi_handler, 0, "PMI");
> [Severity: High]
> Does clamping x86_pmu.lbr_nr after intel_pmu_init() and amd_pmu_init()
> return risk an unhandled #GP in intel_pmu_arch_lbr_reset() and leave
> the vendor PMU initialization inconsistent with the clamped depth?
>
> For Intel Architectural LBR, supported depths are enumerated as a bitmask
> in eax.split.lbr_depth_mask (where each bit indicates support for a
> multiple of 8 up to 64). During init_hw_perf_events() -> intel_pmu_init()
> -> intel_pmu_arch_lbr_init(), the code selects the highest supported bit
> and validates that unclamped depth against MSR_ARCH_LBR_DEPTH:
>
> arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_init() {
> 	...
> 	lbr_nr = fls(eax.split.lbr_depth_mask) * 8;
> 	if (!lbr_nr)
> 		goto clear_arch_lbr;
>
> 	/* Apply the max depth of Arch LBR */
> 	if (wrmsrq_safe(MSR_ARCH_LBR_DEPTH, lbr_nr))
> 		goto clear_arch_lbr;
> 	...
> }
>
> If a CPU or hypervisor advertises a depth greater than 32 in
> lbr_depth_mask without also setting the bit for depth 32, clamping
> x86_pmu.lbr_nr to MAX_LBR_ENTRIES (32) here leaves x86_pmu.lbr_nr set to
> an unsupported depth that was never tested with wrmsrq_safe(). When
> cpuhp_setup_state() subsequently runs x86_pmu_starting_cpu() ->
> intel_pmu_cpu_starting() -> intel_pmu_lbr_reset() ->
> intel_pmu_arch_lbr_reset():
>
> arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_reset() {
> 	/* Write to ARCH_LBR_DEPTH MSR, all LBR entries are reset to 0 */
> 	wrmsrq(MSR_ARCH_LBR_DEPTH, x86_pmu.lbr_nr);
> }
>
> won't this unchecked wrmsrq() of 32 to MSR_ARCH_LBR_DEPTH raise a fatal
> #GP at boot?
>
> Additionally, because this clamp runs after vendor initialization has
> completed:
>
> 1. intel_pmu_arch_lbr_init() has already validated the XSAVE size via
>    is_arch_lbr_xsave_available() and allocated pmu->task_ctx_cache using
>    the unclamped lbr_nr.
>
> 2. intel_pmu_init() has already probed MSRs up to the unclamped
>    x86_pmu.lbr_nr and logged the unclamped depth:
>
> arch/x86/events/intel/core.c:intel_pmu_init() {
> 	...
> 	for (i = 0; i < x86_pmu.lbr_nr; i++) {
> 		if (!(check_msr(x86_pmu.lbr_from + i, 0xffffUL) &&
> 		      check_msr(x86_pmu.lbr_to + i, 0xffffUL)))
> 			x86_pmu.lbr_nr = 0;
> 	}
>
> 	if (x86_pmu.lbr_nr) {
> 		intel_pmu_lbr_init();
>
> 		pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
> 	...
> }
>
> 3. amd_pmu_lbr_init() has likewise already logged the unclamped depth:
>
> arch/x86/events/amd/lbr.c:amd_pmu_lbr_init() {
> 	...
> 	x86_pmu.lbr_nr = ebx.split.lbr_v2_stack_sz;
>
> 	pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
> 	...
> }
>
> Should the MAX_LBR_ENTRIES limit be enforced at the point where
> x86_pmu.lbr_nr (and lbr_depth_mask in intel_pmu_arch_lbr_init()) is
> determined inside the vendor initialization paths instead?

Ok, directly clampling x86_pmu.lbr_nr to MAX_LBR_ENTRIES seems not a good way. The proper way could be to check if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES, if so, just error out the PMU initialization. Thanks.




>
>>  
>  완벽
>

Re: [Patch v3 3/4] perf/x86/intel/lbr: Reset the cached LBR stack in intel_pmu_lbr_reset()

[Mi, Dapeng]

On 10/19/2026 3:30 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] perf/x86/intel/lbr: stale lbr_stack.nr in intel_pmu_arch_lbr_read_xsave()
> --
>
> commit 93eb502f5c2972255761d542db115fed920f52bd
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86/intel/lbr: Reset the cached LBR stack in intel_pmu_lbr_reset()
>
> This commit resets cpuc->lbr_stack.nr in intel_pmu_lbr_reset() and skips
> calling intel_pmu_store_lbr() in intel_pmu_arch_lbr_read_xsave() when
> XFEATURE_MASK_LBR is not set in xsave->header.xfeatures.
>
>> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
>> index 0b446d8912e4e..249db12e927ed 100644
>> --- a/arch/x86/events/intel/lbr.c
>> +++ b/arch/x86/events/intel/lbr.c
>> @@ -193,6 +193,8 @@ void intel_pmu_lbr_reset(void)
>>  {
>>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>>  
>> +	cpuc->lbr_stack.nr = 0;
>> +
>>  	if (!x86_pmu.lbr_nr)
>>  		return;
>>  
>> @@ -1012,7 +1014,8 @@ static void intel_pmu_arch_lbr_read_xsave(struct cpu_hw_events *cpuc)
>>  	}
>>  	xsaves(&xsave->xsave, XFEATURE_MASK_LBR);
>>  
>> -	intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
>> +	if (xsave->header.xfeatures & XFEATURE_MASK_LBR)
>> +		intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
>>  }
>>  
>>  void intel_pmu_lbr_read(void)
> [Severity: High]
> When xsaves() sees the hardware LBR state in its INIT configuration, the
> XSAVE init optimization clears XFEATURE_MASK_LBR in
> xsave->header.xfeatures without writing to xsave->lbr.entries.
>
> If intel_pmu_store_lbr() is skipped here without resetting
> cpuc->lbr_stack.nr = 0, can cpuc->lbr_stack.nr and cpuc->lbr_entries[]
> retain stale branch entries whenever the hardware LBR state enters the
> INIT configuration without going through intel_pmu_lbr_reset()?
>
> For example, during a context switch in LBR call-stack mode,
> __intel_pmu_lbr_restore() restores the incoming task's LBR state without
> calling intel_pmu_lbr_reset():
>
> arch/x86/events/intel/lbr.c:__intel_pmu_lbr_restore() {
> 	if (!has_lbr_callstack_users(ctx) ||
> 	    task_context_opt(ctx)->lbr_stack_state == LBR_NONE) {
> 		intel_pmu_lbr_reset();
> 		return;
> 	}
> 	...
> 	x86_pmu.lbr_restore(ctx);
>
> 	task_context_opt(ctx)->lbr_stack_state = LBR_NONE;
> }
>
> If the previous task populated cpuc->lbr_stack.nr > 0 and the incoming
> task had 0 branches when scheduled out (so xrstors() restores the hardware
> LBR state to the INIT state), a PMI or branch snapshot in the new task
> before new branches are recorded will see !(xsave->header.xfeatures &
> XFEATURE_MASK_LBR) and leave the previous task's cpuc->lbr_stack.nr and
> cpuc->lbr_entries[] in place.
>
> The same situation can also occur within a single task when:
> - RET instructions in call-stack mode pop all hardware LBR entries back to
>   the INIT state after an earlier PMI populated cpuc->lbr_stack.nr,
> - a deep C-state resets the hardware Arch LBR registers to the INIT state
>   (x86_pmu.lbr_deep_c_reset) without calling intel_pmu_lbr_reset(), or
> - intel_pmu_store_pebs_lbrs() populates cpuc->lbr_stack.nr from a PEBS
>   record while the hardware LBR registers are in the INIT state.
>
> In addition to leaking or repeating stale branch entries,
> intel_pmu_lbr_read() then runs intel_pmu_lbr_filter(cpuc) a second time on
> the stale cpuc->lbr_entries[], where e->type already holds a converted
> PERF_BR_* value rather than a raw ARCH_LBR_BR_TYPE_* value, indexing
> arch_lbr_br_type_map[] with the wrong value.
>
> Should intel_pmu_arch_lbr_read_xsave() clear cpuc->lbr_stack.nr = 0 when
> !(xsave->header.xfeatures & XFEATURE_MASK_LBR)?
>
> 	if (xsave->header.xfeatures & XFEATURE_MASK_LBR)
> 		intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
> 	else
> 		cpuc->lbr_stack.nr = 0;

Good catch. Would follow the suggestion. Thanks.


>

Re: [Patch v3 4/4] perf/x86/intel/lbr: Limit branch snapshots to valid entries

[Mi, Dapeng]

On 10/19/2026 3:35 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] perf/x86/intel/lbr: stale LBR data or -ENOENT in intel_pmu_lbr_read() for PEBS
> - [Medium] perf/x86/intel/lbr: unreset lbr_stack.nr in intel_pmu_arch_lbr_read_xsave()
> - [Medium] perf/x86: late lbr_nr clamping in init_hw_perf_events() after arch LBR init
> --
>
> commit da27a6ac96e6d944e33e171c276d0b044be96def
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86/intel/lbr: Limit branch snapshots to valid entries
>
> This patch limits __intel_pmu_snapshot_branch_stack() to copy up to
> cpuc->lbr_stack.nr entries rather than x86_pmu.lbr_nr, and resets
> cpuc->lbr_stack.nr to 0 in intel_pmu_lbr_read() when there are no active
> LBR users or LBR is excluded on the host.
>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index 0a34d674df590..5dfb9e9156e88 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 249db12e927ed..cc0300aba7de0 100644
>> --- a/arch/x86/events/intel/lbr.c
>> +++ b/arch/x86/events/intel/lbr.c
>> @@ -1022,14 +1022,19 @@ void intel_pmu_lbr_read(void)
>>  {
>>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>>  
>> +	if (!cpuc->lbr_users || vlbr_exclude_host()) {
>> +		/* Reset lbr_stack.nr to prevent stale data leakage. */
>> +		cpuc->lbr_stack.nr = 0;
>> +		return;
>> +	}
>> +
>>  	/*
>>  	 * Don't read when all LBRs users are using adaptive PEBS.
>>  	 *
>>  	 * This could be smarter and actually check the event,
>>  	 * but this simple approach seems to work for now.
>>  	 */
>> -	if (!cpuc->lbr_users || vlbr_exclude_host() ||
>> -	    cpuc->lbr_users == cpuc->lbr_pebs_users)
>> +	if (cpuc->lbr_users == cpuc->lbr_pebs_users)
>>  		return;
> [Severity: High]
> Does skipping x86_pmu.lbr_read(cpuc) here when cpuc->lbr_users ==
> cpuc->lbr_pebs_users cause __intel_pmu_snapshot_branch_stack() to return
> 0 entries or copy stale cpuc->lbr_entries[] to BPF programs?
>
> The commit message notes that when all active LBR users are PEBS events,
> the LBR stack is already updated from the PEBS records. However, PEBS LBR
> records are only written into cpuc->lbr_entries[] and cpuc->lbr_stack.nr
> during counter overflow PMI draining:
>
>   handle_pmi_common()
>     -> x86_pmu_drain_pebs()
>       -> setup_pebs_adaptive_sample_data() / setup_arch_pebs_sample_data()
>         -> intel_pmu_store_pebs_lbrs()
>
> In contrast, bpf_get_branch_snapshot() invokes
> intel_pmu_snapshot_branch_stack() or intel_pmu_snapshot_arch_branch_stack()
> synchronously from a software or tracing event outside of any PEBS drain.
> Those functions freeze the hardware LBRs and call
> __intel_pmu_snapshot_branch_stack() -> intel_pmu_lbr_read() to read the
> current hardware LBR registers.
>
> When all active LBR events on the CPU are adaptive PEBS events, returning
> early in intel_pmu_lbr_read() without reading the hardware LBRs or
> updating cpuc->lbr_stack.nr leads to several issues:
>
> 1. If no PEBS PMI has occurred since the last intel_pmu_lbr_reset(),
>    cpuc->lbr_stack.nr is still 0, so __intel_pmu_snapshot_branch_stack()
>    copies 0 entries and bpf_get_branch_snapshot() fails with -ENOENT even
>    though the LBR hardware has been actively recording branches.
>
> 2. If an earlier PEBS PMI occurred, cpuc->lbr_entries[] and
>    cpuc->lbr_stack.nr hold the historical branch stack from that old PEBS
>    overflow sample rather than the live branches frozen at the snapshot
>    point.
>
> 3. Stale or cross-task entries can also remain in cpuc->lbr_entries[] with
>    cpuc->lbr_stack.nr > 0 across transitions that do not call
>    intel_pmu_lbr_reset():
>
>    arch/x86/events/intel/lbr.c:intel_pmu_lbr_add() {
>        if (x86_pmu.intel_cap.pebs_baseline && event->attr.precise_ip > 0)
>            cpuc->lbr_pebs_users++;
>        perf_sched_cb_inc(event->pmu);
>        if (!cpuc->lbr_users++ && !event->total_time_running)
>            intel_pmu_lbr_reset();
>    }
>
>    Scheduling in a previously active PEBS LBR event where
>    event->total_time_running > 0 (or removing a non-PEBS LBR event via
>    intel_pmu_lbr_del() while a PEBS LBR event remains active) does not
>    reset cpuc->lbr_stack.nr. Similarly, on a context switch with
>    PERF_SAMPLE_BRANCH_CALL_STACK:
>
>    arch/x86/events/intel/lbr.c:__intel_pmu_lbr_restore() {
>        ...
>        x86_pmu.lbr_restore(ctx);
>
>        task_context_opt(ctx)->lbr_stack_state = LBR_NONE;
>    }
>
>    __intel_pmu_lbr_restore() updates the hardware LBR MSRs for the
>    incoming task without resetting cpuc->lbr_stack.nr, leaving the
>    previous task's cached cpuc->lbr_entries[] to be copied by
>    __intel_pmu_snapshot_branch_stack().
>
>>  
>>  	x86_pmu.lbr_read(cpuc);

Hmm, I hesitated for a long time on whether perf should snapshot the
runtime LBR registers instead of using the cached LBR stack data which may
comes from the PEBS record previously. If
__intel_pmu_snapshot_branch_stack() doesn't snapshot the runtime hardware
LBR registers, then BPF may get the stale LBR stack instead of the LBR
stack data at the BPF hook point. If __intel_pmu_snapshot_branch_stack()
snapshots the runtime LBR stack, it would overwrite the cached the PEBS LBR
stack data and this data could be used in the regular overflow handler
processing later.

We could have to save the PEBS LBR stack into a temporary buffer, and then
restore them into the cpuc->lbr_stack after copying the runtime hardware
LBR stack to BFP buffer. Thanks. 




> [Severity: Medium]
> Can x86_pmu.lbr_read(cpuc) leave a stale non-zero cpuc->lbr_stack.nr when
> intel_pmu_arch_lbr_read_xsave() sees the hardware Arch LBR state in its
> XSAVE INIT configuration?
>
> In the preceding patch ("perf/x86/intel/lbr: Reset the cached LBR stack in
> intel_pmu_lbr_reset()"), intel_pmu_arch_lbr_read_xsave() was updated to:
>
> arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_read_xsave() {
> 	...
> 	xsaves(&xsave->xsave, XFEATURE_MASK_LBR);
>
> 	if (xsave->header.xfeatures & XFEATURE_MASK_LBR)
> 		intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
> }
>
> When 0 branches have been recorded in hardware, the Arch LBR state is in
> its XSAVE INIT configuration, so xsaves() clears XFEATURE_MASK_LBR in
> xsave->header.xfeatures. Unlike the MSR fallback path
> intel_pmu_store_lbr(cpuc, NULL) which reads 0 from MSR_ARCH_LBR_FROM_0 and
> sets cpuc->lbr_stack.nr = 0, skipping intel_pmu_store_lbr() here without
> an else branch leaves cpuc->lbr_stack.nr at its previous value.
>
> If cpuc->lbr_stack.nr was non-zero from an earlier read and the hardware
> Arch LBR state subsequently returns to the INIT state without
> intel_pmu_lbr_reset() being called (for example, when
> __intel_pmu_lbr_restore() restores a task's call-stack LBR context via
> intel_pmu_arch_lbr_xrstors() whose saved xfeatures has XFEATURE_MASK_LBR
> clear, when RET instructions pop all prior CALL entries in call-stack
> mode, when a CPU wakes from a deep C-state that resets Arch LBR registers,
> or when an LBR event with event->total_time_running > 0 is scheduled back
> in), __intel_pmu_snapshot_branch_stack() will see the old
> cpuc->lbr_stack.nr and copy stale cpuc->lbr_entries[] into entries.
>
> Should intel_pmu_arch_lbr_read_xsave() set cpuc->lbr_stack.nr = 0 when
> !(xsave->header.xfeatures & XFEATURE_MASK_LBR)?
>
>
> [Severity: Medium]
> Also in the earlier patch in this series ("perf/x86: Clamp x86_pmu.lbr_nr
> to MAX_LBR_ENTRIES"), init_hw_perf_events() clamps x86_pmu.lbr_nr after
> vendor PMU initialization has already run:
>
> arch/x86/events/core.c:init_hw_perf_events() {
> 	...
> 	err = x86_pmu.init();
> 	...
> 	x86_pmu.lbr_nr = min(x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
> }
>
> During x86_pmu.init() -> intel_pmu_init() -> intel_pmu_arch_lbr_init(),
> the Arch LBR depth is selected from the highest bit in
> eax.split.lbr_depth_mask (up to 64), tested against MSR_ARCH_LBR_DEPTH,
> and validated against xfeature_size(XFEATURE_LBR):
>
> arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_init() {
> 	...
> 	lbr_nr = fls(eax.split.lbr_depth_mask) * 8;
> 	if (!lbr_nr)
> 		goto clear_arch_lbr;
>
> 	/* Apply the max depth of Arch LBR */
> 	if (wrmsrq_safe(MSR_ARCH_LBR_DEPTH, lbr_nr))
> 		goto clear_arch_lbr;
>
> 	x86_pmu.lbr_nr = lbr_nr;
>
> 	if (has_arch_lbr_xsave() &&
> 	    xfeature_size(XFEATURE_LBR) != get_lbr_state_size()) {
> 		WARN_ON(1);
> 		goto clear_arch_lbr;
> 	}
> }
>
> If hardware enumerates a maximum depth greater than MAX_LBR_ENTRIES (32)
> in eax.split.lbr_depth_mask, clamping x86_pmu.lbr_nr to 32 afterwards in
> init_hw_perf_events() means intel_pmu_arch_lbr_reset() will later execute
> an unchecked wrmsrq(MSR_ARCH_LBR_DEPTH, x86_pmu.lbr_nr) with depth 32:
>
> arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_reset() {
> 	/* Write to ARCH_LBR_DEPTH MSR, all LBR entries are reset to 0 */
> 	wrmsrq(MSR_ARCH_LBR_DEPTH, x86_pmu.lbr_nr);
> }
>
> If depth 32 (bit 3 in eax.split.lbr_depth_mask) is not supported by the
> processor or VM, won't that wrmsrq(MSR_ARCH_LBR_DEPTH, 32) trigger an
> unhandled #GP fault during CPU startup or LBR reset, in addition to
> get_lbr_state_size() and the boot logs having used the unclamped
> x86_pmu.lbr_nr?
>
> Should intel_pmu_arch_lbr_init() mask or bound the selected depth to
> MAX_LBR_ENTRIES before validating MSR_ARCH_LBR_DEPTH and
> get_lbr_state_size()?
>

Re: [Patch v3 4/4] perf/x86/intel/lbr: Limit branch snapshots to valid entries

[Mi, Dapeng]

Sashiko raises new comments about this patch,
https://lore.kernel.org/all/sashiko-outbox-176751@kernel.org/

It looks we need a new version to address the comments. Thanks.

On 10/19/2026 3:08 PM, Dapeng Mi 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
> no active LBR events or LBR hardware is owned by guest. So the stale or
> guest LBR stack data can't be leaked.
>
> When all active LBR users are PEBS events, the LBR stack is already
> updated from the PEBS records, so there is no need to read the LBR
> hardware again.
>
> 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/
> Co-developed-by: Sandipan Das <sandipan.das@amd.com>
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---
>  arch/x86/events/intel/core.c | 2 +-
>  arch/x86/events/intel/lbr.c  | 9 +++++++--
>  2 files changed, 8 insertions(+), 3 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 249db12e927e..cc0300aba7de 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1022,14 +1022,19 @@ void intel_pmu_lbr_read(void)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>  
> +	if (!cpuc->lbr_users || vlbr_exclude_host()) {
> +		/* Reset lbr_stack.nr to prevent stale data leakage. */
> +		cpuc->lbr_stack.nr = 0;
> +		return;
> +	}
> +
>  	/*
>  	 * Don't read when all LBRs users are using adaptive PEBS.
>  	 *
>  	 * This could be smarter and actually check the event,
>  	 * but this simple approach seems to work for now.
>  	 */
> -	if (!cpuc->lbr_users || vlbr_exclude_host() ||
> -	    cpuc->lbr_users == cpuc->lbr_pebs_users)
> +	if (cpuc->lbr_users == cpuc->lbr_pebs_users)
>  		return;
>  
>  	x86_pmu.lbr_read(cpuc);