[Patch v4 0/5] perf/x86: Limit branch snapshots to valid entries

[Patch v4 0/5] 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 v4:
- Patch 01: Disable LBR if parsed x86_pmu.lbr_nr exceeds
  MAX_LBR_ENTRIES (Sashiko).
- Patch 05: New patch, which always snapshots live hardware LBR stack
  for BPF path (Sashiko).

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:
v3: https://lore.kernel.org/all/20261019070836.2972403-1-dapeng1.mi@linux.intel.com/
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 (4):
  perf/x86: Disable LBR if x86_pmu.lbr_nr exceeds 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
  pef/x86/intel: Always snapshot live hardware LBR state for BPF path

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       |  6 +++++
 arch/x86/events/intel/core.c | 45 +++++++++++++++++++++++++++++++++---
 arch/x86/events/intel/lbr.c  | 19 +++++++--------
 5 files changed, 62 insertions(+), 14 deletions(-)


base-commit: f5a78ecccd98a04c47cea1cd63c2a2fdce8a4b48
-- 
2.34.1

[Patch v4 1/5] perf/x86: Disable LBR if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES

[Dapeng Mi]

Disable LBR if x86_pmu.lbr_nr is larger than MAX_LBR_ENTRIES, preventing
out-of-bounds access in the cached LBR stack handling path.

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

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 4df792b139e0..002a04c149c2 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2645,6 +2645,12 @@ static int __init init_hw_perf_events(void)
 	if (!x86_pmu.config_mask)
 		x86_pmu.config_mask = X86_RAW_EVENT_MASK;
 
+	if (x86_pmu.lbr_nr > MAX_LBR_ENTRIES) {
+		pr_warn("LBR entries %d exceeds max %d, disable LBR.\n",
+			x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
+		x86_pmu.lbr_nr = 0;
+	}
+
 	perf_events_lapic_init();
 	register_nmi_handler(NMI_LOCAL, perf_event_nmi_handler, 0, "PMI");
 
-- 
2.34.1

Re: [Patch v4 1/5] perf/x86: Disable LBR if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf/x86: init_hw_perf_events() disables LBR too late after vendor PMU init
--

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

perf/x86: Disable LBR if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES

This commit disables last branch record support in init_hw_perf_events()
if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES to prevent out-of-bounds
accesses in the cached branch stack handling path.

> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 4df792b139e05..002a04c149c29 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -2645,6 +2645,12 @@ static int __init init_hw_perf_events(void)
>  	if (!x86_pmu.config_mask)
>  		x86_pmu.config_mask = X86_RAW_EVENT_MASK;
>  
> +	if (x86_pmu.lbr_nr > MAX_LBR_ENTRIES) {
> +		pr_warn("LBR entries %d exceeds max %d, disable LBR.\n",
> +			x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
> +		x86_pmu.lbr_nr = 0;
> +	}
> +
>  	perf_events_lapic_init();
>  	register_nmi_handler(NMI_LOCAL, perf_event_nmi_handler, 0, "PMI");

[Severity: Medium]
Does zeroing x86_pmu.lbr_nr here in init_hw_perf_events() happen too late,
after intel_pmu_init() and amd_pmu_init() have already configured hardware
and driver state using the unvalidated x86_pmu.lbr_nr value?

In arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_init(), if lbr_nr exceeds
MAX_LBR_ENTRIES, initialization proceeds instead of taking the clear_arch_lbr
path:

	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_counters = ecx.split.lbr_counters;
	x86_pmu.lbr_nr = lbr_nr;

	if (!!x86_pmu.lbr_counters)
		x86_pmu.flags |= PMU_FL_BR_CNTR | PMU_FL_DYN_CONSTRAINT;

Because clear_arch_lbr is skipped, X86_FEATURE_ARCH_LBR remains enabled,
MSR_ARCH_LBR_DEPTH is written, pmu->task_ctx_cache is allocated via
create_lbr_kmem_cache(), and x86_pmu.lbr_ctl_map and static branches are
configured.

Subsequently, arch/x86/events/intel/lbr.c:reserve_lbr_buffers() checks the
feature bit and task_ctx_cache rather than x86_pmu.lbr_nr:

	if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR))
		return;

	for_each_possible_cpu(cpu) {
		cpuc = per_cpu_ptr(&cpu_hw_events, cpu);
		kmem_cache = x86_get_pmu(cpu)->task_ctx_cache;
		if (!kmem_cache || cpuc->lbr_xsave)
			continue;

		cpuc->lbr_xsave = kmem_cache_alloc_node(kmem_cache,
							GFP_KERNEL | __GFP_ZERO,
							cpu_to_node(cpu));
	}

Wouldn't this still allocate per-cpu cpuc->lbr_xsave buffers on hardware
reservation even after x86_pmu.lbr_nr is set to 0?

Similarly, in arch/x86/events/intel/core.c:lbr_is_visible(), leaving
PMU_FL_BR_CNTR set in x86_pmu.flags keeps branch_counter_nr and
branch_counter_width visible in sysfs when x86_pmu.lbr_nr is 0:

	/* branches */
	if (i == 0)
		return x86_pmu.lbr_nr ? attr->mode : 0;

	return (x86_pmu.flags & PMU_FL_BR_CNTR) ? attr->mode : 0;

In addition, both arch/x86/events/intel/core.c:intel_pmu_init() and
arch/x86/events/amd/core.c:amd_core_pmu_init() check x86_pmu.lbr_nr before
init_hw_perf_events() clears it:

	if (x86_pmu.lbr_nr) {
		intel_pmu_lbr_init();

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

		/* only support branch_stack snapshot for perfmon >= v2 */
		if (x86_pmu.disable_all == intel_pmu_disable_all) {
			if (boot_cpu_has(X86_FEATURE_ARCH_LBR)) {
				static_call_update(perf_snapshot_branch_stack,
						   intel_pmu_snapshot_arch_branch_stack);
			} else {
				static_call_update(perf_snapshot_branch_stack,
						   intel_pmu_snapshot_branch_stack);
			}
		}
	}

And in amd_core_pmu_init():

	if (!amd_pmu_lbr_init()) {
		/* LBR requires flushing on context switch */
		x86_pmu.sched_task = amd_pmu_lbr_sched_task;
		static_call_update(amd_pmu_branch_hw_config, amd_pmu_lbr_hw_config);
		static_call_update(amd_pmu_branch_reset, amd_pmu_lbr_reset);
		static_call_update(amd_pmu_branch_add, amd_pmu_lbr_add);
		static_call_update(amd_pmu_branch_del, amd_pmu_lbr_del);

		/* Only support branch_stack snapshot on perfmon v2 */
		if (x86_pmu.handle_irq == amd_pmu_v2_handle_irq)
			static_call_update(perf_snapshot_branch_stack, amd_pmu_v2_snapshot_branch_stack);
	} else if (!amd_brs_init()) {

If bpf_get_branch_snapshot() is later called when x86_pmu.lbr_nr is 0, won't
the registered perf_snapshot_branch_stack callback still run, manipulate
branch and global control registers, and unconditionally call
intel_pmu_enable_all(0) or amd_pmu_v2_enable_all(0)?

Should the MAX_LBR_ENTRIES check be performed before or during the
vendor-specific initialization routines (such as intel_pmu_lbr_init(),
intel_pmu_arch_lbr_init(), and amd_pmu_lbr_init()) so that None of the
hardware state, slab caches, flags, or static calls are enabled when
lbr_nr is out of range?

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

Re: [Patch v4 1/5] perf/x86: Disable LBR if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES

[Mi, Dapeng]

On 10/21/2026 1:42 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] perf/x86: init_hw_perf_events() disables LBR too late after vendor PMU init
> --
>
> commit c6c1bbb1f4feb929dd247ddd507177ce58d871e1
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86: Disable LBR if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES
>
> This commit disables last branch record support in init_hw_perf_events()
> if x86_pmu.lbr_nr exceeds MAX_LBR_ENTRIES to prevent out-of-bounds
> accesses in the cached branch stack handling path.
>
>> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
>> index 4df792b139e05..002a04c149c29 100644
>> --- a/arch/x86/events/core.c
>> +++ b/arch/x86/events/core.c
>> @@ -2645,6 +2645,12 @@ static int __init init_hw_perf_events(void)
>>  	if (!x86_pmu.config_mask)
>>  		x86_pmu.config_mask = X86_RAW_EVENT_MASK;
>>  
>> +	if (x86_pmu.lbr_nr > MAX_LBR_ENTRIES) {
>> +		pr_warn("LBR entries %d exceeds max %d, disable LBR.\n",
>> +			x86_pmu.lbr_nr, MAX_LBR_ENTRIES);
>> +		x86_pmu.lbr_nr = 0;
>> +	}
>> +
>>  	perf_events_lapic_init();
>>  	register_nmi_handler(NMI_LOCAL, perf_event_nmi_handler, 0, "PMI");
> [Severity: Medium]
> Does zeroing x86_pmu.lbr_nr here in init_hw_perf_events() happen too late,
> after intel_pmu_init() and amd_pmu_init() have already configured hardware
> and driver state using the unvalidated x86_pmu.lbr_nr value?
>
> In arch/x86/events/intel/lbr.c:intel_pmu_arch_lbr_init(), if lbr_nr exceeds
> MAX_LBR_ENTRIES, initialization proceeds instead of taking the clear_arch_lbr
> path:
>
> 	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_counters = ecx.split.lbr_counters;
> 	x86_pmu.lbr_nr = lbr_nr;
>
> 	if (!!x86_pmu.lbr_counters)
> 		x86_pmu.flags |= PMU_FL_BR_CNTR | PMU_FL_DYN_CONSTRAINT;
>
> Because clear_arch_lbr is skipped, X86_FEATURE_ARCH_LBR remains enabled,
> MSR_ARCH_LBR_DEPTH is written, pmu->task_ctx_cache is allocated via
> create_lbr_kmem_cache(), and x86_pmu.lbr_ctl_map and static branches are
> configured.
>
> Subsequently, arch/x86/events/intel/lbr.c:reserve_lbr_buffers() checks the
> feature bit and task_ctx_cache rather than x86_pmu.lbr_nr:
>
> 	if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR))
> 		return;
>
> 	for_each_possible_cpu(cpu) {
> 		cpuc = per_cpu_ptr(&cpu_hw_events, cpu);
> 		kmem_cache = x86_get_pmu(cpu)->task_ctx_cache;
> 		if (!kmem_cache || cpuc->lbr_xsave)
> 			continue;
>
> 		cpuc->lbr_xsave = kmem_cache_alloc_node(kmem_cache,
> 							GFP_KERNEL | __GFP_ZERO,
> 							cpu_to_node(cpu));
> 	}
>
> Wouldn't this still allocate per-cpu cpuc->lbr_xsave buffers on hardware
> reservation even after x86_pmu.lbr_nr is set to 0?
>
> Similarly, in arch/x86/events/intel/core.c:lbr_is_visible(), leaving
> PMU_FL_BR_CNTR set in x86_pmu.flags keeps branch_counter_nr and
> branch_counter_width visible in sysfs when x86_pmu.lbr_nr is 0:
>
> 	/* branches */
> 	if (i == 0)
> 		return x86_pmu.lbr_nr ? attr->mode : 0;
>
> 	return (x86_pmu.flags & PMU_FL_BR_CNTR) ? attr->mode : 0;
>
> In addition, both arch/x86/events/intel/core.c:intel_pmu_init() and
> arch/x86/events/amd/core.c:amd_core_pmu_init() check x86_pmu.lbr_nr before
> init_hw_perf_events() clears it:
>
> 	if (x86_pmu.lbr_nr) {
> 		intel_pmu_lbr_init();
>
> 		pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
>
> 		/* only support branch_stack snapshot for perfmon >= v2 */
> 		if (x86_pmu.disable_all == intel_pmu_disable_all) {
> 			if (boot_cpu_has(X86_FEATURE_ARCH_LBR)) {
> 				static_call_update(perf_snapshot_branch_stack,
> 						   intel_pmu_snapshot_arch_branch_stack);
> 			} else {
> 				static_call_update(perf_snapshot_branch_stack,
> 						   intel_pmu_snapshot_branch_stack);
> 			}
> 		}
> 	}
>
> And in amd_core_pmu_init():
>
> 	if (!amd_pmu_lbr_init()) {
> 		/* LBR requires flushing on context switch */
> 		x86_pmu.sched_task = amd_pmu_lbr_sched_task;
> 		static_call_update(amd_pmu_branch_hw_config, amd_pmu_lbr_hw_config);
> 		static_call_update(amd_pmu_branch_reset, amd_pmu_lbr_reset);
> 		static_call_update(amd_pmu_branch_add, amd_pmu_lbr_add);
> 		static_call_update(amd_pmu_branch_del, amd_pmu_lbr_del);
>
> 		/* Only support branch_stack snapshot on perfmon v2 */
> 		if (x86_pmu.handle_irq == amd_pmu_v2_handle_irq)
> 			static_call_update(perf_snapshot_branch_stack, amd_pmu_v2_snapshot_branch_stack);
> 	} else if (!amd_brs_init()) {
>
> If bpf_get_branch_snapshot() is later called when x86_pmu.lbr_nr is 0, won't
> the registered perf_snapshot_branch_stack callback still run, manipulate
> branch and global control registers, and unconditionally call
> intel_pmu_enable_all(0) or amd_pmu_v2_enable_all(0)?
>
> Should the MAX_LBR_ENTRIES check be performed before or during the
> vendor-specific initialization routines (such as intel_pmu_lbr_init(),
> intel_pmu_arch_lbr_init(), and amd_pmu_lbr_init()) so that None of the
> hardware state, slab caches, flags, or static calls are enabled when
> lbr_nr is out of range?

The change is good enough. The branch sampling perf event creation is
guarded with x86_pmu.lbr_nr regardless of on Intel or AMD platforms. If
x86_pmu.lbr_nr is 0, then no events with branch sampling can be created
successfully, and if no active LBR event is present, the LBR hardware is
not really enabled.

Since it's a common validation for all x86 vendor platforms, it should be
placed into the x86 common PMU initialization path.

Thanks.


>

[Patch v4 2/5] 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

Re: [Patch v4 2/5] 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/20261021050711.4043703-1-dapeng1.mi@linux.intel.com?part=2

[Patch v4 3/5] 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.

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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 0b446d8912e4..c7e295db9410 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,10 @@ 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);
+	else
+		cpuc->lbr_stack.nr = 0;
 }
 
 void intel_pmu_lbr_read(void)
-- 
2.34.1

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

[sashiko-bot]

> 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.
> 
> Fixes: c085fb877467 ("perf/x86/intel/lbr: Support XSAVES for arch LBR read")
> 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/20261021050711.4043703-1-dapeng1.mi@linux.intel.com?part=3

[Patch v4 4/5] 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.

For the case where all LBR events are PEBS events, hardware LBR reads are
currently skipped; that can leave stale LBR stack data from previous PEBS
sampling in place and may be inconsistent with the intended semantics of
bpf_get_branch_snapshot(). This will be addressed in a follow-up patch.

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 c7e295db9410..ac1759a71288 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1024,14 +1024,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 v4 4/5] perf/x86/intel/lbr: Limit branch snapshots to valid entries

[sashiko-bot]

> 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.
> 
> For the case where all LBR events are PEBS events, hardware LBR reads are
> currently skipped; that can leave stale LBR stack data from previous PEBS
> sampling in place and may be inconsistent with the intended semantics of
> bpf_get_branch_snapshot(). This will be addressed in a follow-up patch.
> 
> 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>

Sashiko has reviewed this patch and found no issues.

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

[Patch v4 5/5] pef/x86/intel: Always snapshot live hardware LBR state for BPF path

[Dapeng Mi]

__intel_pmu_snapshot_branch_stack() is meant to snapshot the current
runtime LBR state from hardware for the BPF path. However, stale cached
PEBS LBR data can be snapshotted instead when a PEBS LBR user is active.

Fix this by always snapshotting the live hardware LBR state, regardless
of whether a PEBS LBR user is active. Reading the runtime LBR registers
can overwrite the cached PEBS LBR stack, so save and restore the cached
PEBS LBR state around the snapshot. This preserves the PEBS LBR data for
later output to userspace in the normal PEBS record path.

Fixes: c22ac2a3d4bd ("perf: Enable branch record for software events")
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 arch/x86/events/intel/core.c | 43 ++++++++++++++++++++++++++++++++++--
 arch/x86/events/intel/lbr.c  |  9 --------
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 5dfb9e9156e8..07d1a8a7845b 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2815,16 +2815,50 @@ static void intel_pmu_enable_all(int added)
 	__intel_pmu_enable_all(added, false);
 }
 
+struct lbr_snapshot {
+	struct perf_branch_stack stack;
+	struct perf_branch_entry entries[MAX_LBR_ENTRIES];
+	u64 counters[MAX_LBR_ENTRIES];
+};
+static DEFINE_PER_CPU(struct lbr_snapshot, lbr_snapshot);
+
 static noinline int
 __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
 				  unsigned int cnt, unsigned long flags)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+	struct lbr_snapshot *snapshot = this_cpu_ptr(&lbr_snapshot);
+
+	/*
+	 * This may be called from bpf_event_handler() in a PMI context. If
+	 * a PEBS LBR user is active, the PEBS sample record has already updated
+	 * cpuc->lbr_stack. Reading the LBR registers here would overwrite that
+	 * cached PEBS LBR stack, so save it first and restore it afterward. This
+	 * keeps the cached LBR data available for later output to userspace in
+	 * the normal PEBS record path.
+	 */
+	if (cpuc->lbr_pebs_users) {
+		memcpy(&snapshot->stack, &cpuc->lbr_stack,
+		       sizeof(struct perf_branch_stack));
+		memcpy(snapshot->entries, cpuc->lbr_entries,
+		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
+		memcpy(snapshot->counters, cpuc->lbr_counters,
+		       sizeof(u64) * cpuc->lbr_stack.nr);
+	}
 
 	intel_pmu_lbr_read();
 	cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
-
 	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
+
+	if (cpuc->lbr_pebs_users) {
+		memcpy(&cpuc->lbr_stack, &snapshot->stack,
+		       sizeof(struct perf_branch_stack));
+		memcpy(cpuc->lbr_entries, snapshot->entries,
+		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
+		memcpy(cpuc->lbr_counters, snapshot->counters,
+		       sizeof(u64) * cpuc->lbr_stack.nr);
+	}
+
 	intel_pmu_enable_all(0);
 	local_irq_restore(flags);
 	return cnt;
@@ -4022,7 +4056,12 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 
 	loops = 0;
 again:
-	intel_pmu_lbr_read();
+	/*
+	 * No need to read the LBR stack when all LBR users are PEBS events,
+	 * since the PEBS handler updates it.
+	 */
+	if (cpuc->lbr_users != cpuc->lbr_pebs_users)
+		intel_pmu_lbr_read();
 	intel_pmu_ack_status(status);
 	if (++loops > 100) {
 		static bool warned;
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index ac1759a71288..07b32255e48d 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1030,15 +1030,6 @@ void intel_pmu_lbr_read(void)
 		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 == cpuc->lbr_pebs_users)
-		return;
-
 	x86_pmu.lbr_read(cpuc);
 
 	intel_pmu_lbr_filter(cpuc);
-- 
2.34.1

Re: [Patch v4 5/5] pef/x86/intel: Always snapshot live hardware LBR state for BPF path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] perf/x86/intel: LBR state corruption in __intel_pmu_snapshot_branch_stack()
- [High] perf/x86/intel: __intel_pmu_snapshot_branch_stack() enables PMU in PMI
- [Medium] perf/x86/intel: unfrozen LBRs and PMU re-enable in PMI branch snapshot
- [Medium] perf/x86: NMI race on lbr_snapshot in __intel_pmu_snapshot_branch_stack()
- [Medium] perf/x86/intel: unfiltered LBR branch counter bits in perf_branch_entry.reserved
--

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

pef/x86/intel: Always snapshot live hardware LBR state for BPF path

This commit updates __intel_pmu_snapshot_branch_stack() to always read live
hardware LBR registers for the BPF snapshot path even when PEBS LBR users
are active. To avoid clobbering cached PEBS LBR data during a PMI, it saves
and restores cpuc->lbr_stack, cpuc->lbr_entries, and cpuc->lbr_counters
around intel_pmu_lbr_read() using a per-CPU buffer, and moves the
cpuc->lbr_users == cpuc->lbr_pebs_users check into intel_pmu_handle_irq().

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 5dfb9e9156e88..07d1a8a7845b2 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -2815,16 +2815,50 @@ static void intel_pmu_enable_all(int added)
>  	__intel_pmu_enable_all(added, false);
>  }
>  
> +struct lbr_snapshot {
> +	struct perf_branch_stack stack;
> +	struct perf_branch_entry entries[MAX_LBR_ENTRIES];
> +	u64 counters[MAX_LBR_ENTRIES];
> +};
> +static DEFINE_PER_CPU(struct lbr_snapshot, lbr_snapshot);

[Severity: Medium]
Can an NMI race on the per-CPU lbr_snapshot buffer when
__intel_pmu_snapshot_branch_stack() is called in non-NMI context?

In arch/x86/events/intel/core.c, intel_pmu_snapshot_branch_stack() and
intel_pmu_snapshot_arch_branch_stack() call local_irq_save(flags) and
__intel_pmu_disable_all(false) before invoking
__intel_pmu_snapshot_branch_stack(). However, local_irq_save() only masks
normal interrupts, and an in-flight PMI or another NMI can still arrive
after __intel_pmu_disable_all() due to NMI delivery latency.

When cpuc->lbr_pebs_users > 0 and a non-NMI BPF program (such as an
fentry/fexit trampoline via __bpf_prog_enter_recur() or a raw_tracepoint,
where bpf_prog_active is not incremented) calls:

  bpf_get_branch_snapshot()
    -> intel_pmu_snapshot_branch_stack()
      -> __intel_pmu_snapshot_branch_stack()

an arriving PMI can still invoke bpf_overflow_handler() ->
bpf_get_branch_snapshot() -> __intel_pmu_snapshot_branch_stack() on the
same CPU, overwriting this_cpu_ptr(&lbr_snapshot) while the outer call is
between its save and restore steps. Even if the PMI does not call
bpf_get_branch_snapshot(), intel_pmu_handle_irq() updates cpuc->lbr_stack
and cpuc->lbr_entries while the interrupted snapshot is saving or
restoring them.

Since cpuc->lbr_stack only holds live sample data awaiting output during a
PMI, should the save and restore of cpuc->lbr_stack be restricted to NMI
context or otherwise protected against NMI reentrancy?

> +
>  static noinline int
>  __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
>  				  unsigned int cnt, unsigned long flags)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> +	struct lbr_snapshot *snapshot = this_cpu_ptr(&lbr_snapshot);
> +
> +	/*
> +	 * This may be called from bpf_event_handler() in a PMI context. If
> +	 * a PEBS LBR user is active, the PEBS sample record has already updated
> +	 * cpuc->lbr_stack. Reading the LBR registers here would overwrite that
> +	 * cached PEBS LBR stack, so save it first and restore it afterward. This
> +	 * keeps the cached LBR data available for later output to userspace in
> +	 * the normal PEBS record path.
> +	 */
> +	if (cpuc->lbr_pebs_users) {
> +		memcpy(&snapshot->stack, &cpuc->lbr_stack,
> +		       sizeof(struct perf_branch_stack));
> +		memcpy(snapshot->entries, cpuc->lbr_entries,
> +		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
> +		memcpy(snapshot->counters, cpuc->lbr_counters,
> +		       sizeof(u64) * cpuc->lbr_stack.nr);
> +	}

[Severity: Critical]
What happens in __intel_pmu_snapshot_branch_stack() when cpuc->lbr_users
is non-zero, but cpuc->lbr_pebs_users is 0?

In arch/x86/events/intel/lbr.c:intel_pmu_lbr_add(), cpuc->lbr_pebs_users
is only incremented when x86_pmu.intel_cap.pebs_baseline is set and
event->attr.precise_ip > 0:

	if (x86_pmu.intel_cap.pebs_baseline && event->attr.precise_ip > 0)
		cpuc->lbr_pebs_users++;

When non-PEBS hardware LBR events (precise_ip == 0), legacy PEBS LBR
events (pre-Ice Lake where pebs_baseline == 0), or Architectural PEBS LBR
events (arch/x86/events/intel/ds.c:setup_arch_pebs_sample_data(), where
pebs_baseline == 0) are active, cpuc->lbr_users > 0 while
cpuc->lbr_pebs_users == 0.

In those paths, handle_pmi_common(), setup_pebs_fixed_sample_data(), and
setup_arch_pebs_sample_data() call intel_pmu_lbr_save_brstack() before
perf_event_overflow() -> __perf_event_overflow() ->
bpf_overflow_handler():

	if (has_branch_stack(event))
		intel_pmu_lbr_save_brstack(&data, cpuc, event);
	...
	perf_event_overflow(event, &data, regs);

Inside include/linux/perf_event.h:perf_sample_save_brstack(), brs
(&cpuc->lbr_stack) is clamped in place and used to precompute
data->dyn_size:

	brs->nr = min_t(u16, event->attr.sample_max_stack, brs->nr);
	size += brs->nr * sizeof(struct perf_branch_entry);
	if (brs_cntr)
		size += brs->nr * sizeof(u64);
	data->br_stack = brs;
	data->br_stack_cntr = brs_cntr;
	data->dyn_size += size;
	data->sample_flags |= PERF_SAMPLE_BRANCH_STACK;

If bpf_overflow_handler() then calls bpf_get_branch_snapshot() ->
__intel_pmu_snapshot_branch_stack() with cpuc->lbr_pebs_users == 0, the
save and restore of cpuc->lbr_stack, cpuc->lbr_entries, and
cpuc->lbr_counters are skipped, and intel_pmu_lbr_read() overwrites them
in place:

1. In kernel/events/core.c:perf_output_sample(), data->br_stack still
   points to &cpuc->lbr_stack. Because intel_pmu_lbr_read() resets
   cpuc->lbr_stack.nr up to x86_pmu.lbr_nr, undoing the sample_max_stack
   clamp after data->dyn_size was already computed, perf_output_sample()
   can copy more perf_branch_entry (and counter) elements than
   perf_output_begin() reserved in the ring buffer.

2. The frozen LBR entries captured at PMI entry (or from the Arch PEBS
   record) in cpuc->lbr_entries are overwritten by unfrozen post-ack LBR
   registers containing PMI handler branches.

3. For branch counter groups, intel_pmu_lbr_counters_reorder() has already
   extracted counter bits from cpuc->lbr_entries[i].reserved into
   cpuc->lbr_counters[i] and cleared .reserved; calling
   intel_pmu_lbr_read() -> intel_pmu_store_lbr() writes raw counter bits
   back into cpuc->lbr_entries[i].reserved and desynchronizes
   cpuc->lbr_counters[].

Should the save and restore in __intel_pmu_snapshot_branch_stack() be
gated on cpuc->lbr_users instead of cpuc->lbr_pebs_users (and save/restore
based on the pre-clamp entry count or full array when needed)?

>  
>  	intel_pmu_lbr_read();
>  	cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
> -
>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);

[Severity: Medium]
On platforms with Architectural LBR and branch counter logging, does
copying cpuc->lbr_entries directly after intel_pmu_lbr_read() leak raw
branch counter bits in entries[i].reserved to BPF programs?

When intel_pmu_snapshot_arch_branch_stack() calls
__intel_pmu_snapshot_branch_stack(), intel_pmu_lbr_read() invokes
intel_pmu_arch_lbr_read() or intel_pmu_arch_lbr_read_xsave(), which calls
arch/x86/events/intel/lbr.c:intel_pmu_store_lbr():

	/*
	 * Leverage the reserved field of
	 * cpuc->lbr_entries[i] to temporarily store the
	 * branch counters information. The later code will
	 * decide what content can be disclosed to the perf
	 * tool. Pleae see intel_pmu_lbr_counters_reorder().
	 */
	.reserved = (info >> LBR_INFO_BR_CNTR_OFFSET) &
		    LBR_INFO_BR_CNTR_FULL_MASK,

Normally intel_pmu_lbr_counters_reorder() filters those bits and clears
cpuc->lbr_entries[i].reserved to 0 before the entries are exposed. Since
__intel_pmu_snapshot_branch_stack() copies cpuc->lbr_entries directly into
entries without calling intel_pmu_lbr_counters_reorder(), won't the raw
hardware counter bits remain in the UAPI perf_branch_entry.reserved field
returned by bpf_get_branch_snapshot()?

> +
> +	if (cpuc->lbr_pebs_users) {
> +		memcpy(&cpuc->lbr_stack, &snapshot->stack,
> +		       sizeof(struct perf_branch_stack));
> +		memcpy(cpuc->lbr_entries, snapshot->entries,
> +		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
> +		memcpy(cpuc->lbr_counters, snapshot->counters,
> +		       sizeof(u64) * cpuc->lbr_stack.nr);
> +	}
> +
>  	intel_pmu_enable_all(0);

[Severity: High]
Is it safe for __intel_pmu_snapshot_branch_stack() to unconditionally call
intel_pmu_enable_all(0) when invoked from bpf_overflow_handler() in a PMI?

In arch/x86/events/intel/core.c:intel_pmu_handle_irq(), the PMI handler
saves cpuc->enabled, sets cpuc->enabled = 0, and disables the PMU in
hardware:

	pmu_enabled = cpuc->enabled;
	...
	cpuc->enabled = 0;
	__intel_pmu_disable_all(true);

The PMI handler expects the PMU, PEBS, BTS, and LBRs to remain disabled in
hardware while handle_pmi_common() and __intel_pmu_pebs_last_event() drain
PEBS records and reprogram counter periods, only restoring the hardware
state at the done label if pmu_enabled was set:

	/* Only restore PMU state when it's active. See x86_pmu_disable(). */
	cpuc->enabled = pmu_enabled;
	if (pmu_enabled)
		__intel_pmu_enable_all(0, true);

When a BPF program runs via perf_event_overflow() ->
bpf_overflow_handler() during a PMI and calls bpf_get_branch_snapshot() ->
__intel_pmu_snapshot_branch_stack(), calling intel_pmu_enable_all(0)
invokes x86_pmu_pebs_enable_all() and __intel_pmu_enable_all(0, false)
while cpuc->enabled is 0. This re-enables MSR_IA32_PEBS_ENABLE, LBRs, BTS,
and MSR_CORE_PERF_GLOBAL_CTRL before __intel_pmu_pebs_last_event() and
handle_pmi_common() finish reprogramming counter periods, and leaves the
hardware PMU enabled after the PMI exits if the PMI interrupted an
x86_pmu_disable() section where pmu_enabled was 0.

Should __intel_pmu_snapshot_branch_stack() avoid calling
intel_pmu_enable_all(0) when cpuc->enabled is 0?

>  	local_irq_restore(flags);
>  	return cnt;
> @@ -4022,7 +4056,12 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
>  
>  	loops = 0;
>  again:
> -	intel_pmu_lbr_read();
> +	/*
> +	 * No need to read the LBR stack when all LBR users are PEBS events,
> +	 * since the PEBS handler updates it.
> +	 */
> +	if (cpuc->lbr_users != cpuc->lbr_pebs_users)
> +		intel_pmu_lbr_read();
>  	intel_pmu_ack_status(status);

[Severity: Medium]
When cpuc->lbr_users == cpuc->lbr_pebs_users and a BPF program calls
bpf_get_branch_snapshot() from bpf_overflow_handler() during a PMI, haven't
the hardware LBR registers already been unfrozen by
intel_pmu_ack_status(status)?

In arch/x86/events/intel/core.c:intel_pmu_handle_irq(),
__intel_pmu_disable_all(true) clears MSR_CORE_PERF_GLOBAL_CTRL but does
not clear LBR enable bits in MSR_IA32_DEBUGCTLMSR or MSR_ARCH_LBR_CTL,
relying on GLOBAL_STATUS_LBRS_FROZEN to keep the LBR stack frozen.

As noted in arch/x86/events/intel/lbr.c:__intel_pmu_lbr_enable():

	/*
	 * No need to unfreeze manually, as v4 can do that as part
	 * of the GLOBAL_STATUS ack.
	 */
	if (pmi && x86_pmu.version >= 4)
		return;

On PMU v4+ platforms (which include all pebs_baseline platforms), calling
intel_pmu_ack_status(status) clears GLOBAL_STATUS_LBRS_FROZEN and
immediately unfreezes the hardware LBRs. By the time handle_pmi_common() ->
x86_pmu_drain_pebs() -> __intel_pmu_pebs_last_event() sets up the PEBS
sample and invokes bpf_overflow_handler() ->
__intel_pmu_snapshot_branch_stack(), the unfrozen hardware LBR MSRs have
been recording branches inside the PMI handler itself (and
__intel_pmu_snapshot_branch_stack() then also calls intel_pmu_enable_all(0)
before the PMI finishes).

Can __intel_pmu_snapshot_branch_stack() still obtain an unpolluted hardware
LBR snapshot in this PMI path after intel_pmu_ack_status(status) has
unfrozen the LBRs?

[ ... ]

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

Re: [Patch v4 5/5] pef/x86/intel: Always snapshot live hardware LBR state for BPF path

[Mi, Dapeng]

On 10/21/2026 2:05 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [Critical] perf/x86/intel: LBR state corruption in __intel_pmu_snapshot_branch_stack()
> - [High] perf/x86/intel: __intel_pmu_snapshot_branch_stack() enables PMU in PMI
> - [Medium] perf/x86/intel: unfrozen LBRs and PMU re-enable in PMI branch snapshot
> - [Medium] perf/x86: NMI race on lbr_snapshot in __intel_pmu_snapshot_branch_stack()
> - [Medium] perf/x86/intel: unfiltered LBR branch counter bits in perf_branch_entry.reserved
> --
>
> commit 4d2b360d61a8a8f6825b3cd919182ee99dbe2d76
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> pef/x86/intel: Always snapshot live hardware LBR state for BPF path
>
> This commit updates __intel_pmu_snapshot_branch_stack() to always read live
> hardware LBR registers for the BPF snapshot path even when PEBS LBR users
> are active. To avoid clobbering cached PEBS LBR data during a PMI, it saves
> and restores cpuc->lbr_stack, cpuc->lbr_entries, and cpuc->lbr_counters
> around intel_pmu_lbr_read() using a per-CPU buffer, and moves the
> cpuc->lbr_users == cpuc->lbr_pebs_users check into intel_pmu_handle_irq().
>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index 5dfb9e9156e88..07d1a8a7845b2 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -2815,16 +2815,50 @@ static void intel_pmu_enable_all(int added)
>>  	__intel_pmu_enable_all(added, false);
>>  }
>>  
>> +struct lbr_snapshot {
>> +	struct perf_branch_stack stack;
>> +	struct perf_branch_entry entries[MAX_LBR_ENTRIES];
>> +	u64 counters[MAX_LBR_ENTRIES];
>> +};
>> +static DEFINE_PER_CPU(struct lbr_snapshot, lbr_snapshot);
> [Severity: Medium]
> Can an NMI race on the per-CPU lbr_snapshot buffer when
> __intel_pmu_snapshot_branch_stack() is called in non-NMI context?
>
> In arch/x86/events/intel/core.c, intel_pmu_snapshot_branch_stack() and
> intel_pmu_snapshot_arch_branch_stack() call local_irq_save(flags) and
> __intel_pmu_disable_all(false) before invoking
> __intel_pmu_snapshot_branch_stack(). However, local_irq_save() only masks
> normal interrupts, and an in-flight PMI or another NMI can still arrive
> after __intel_pmu_disable_all() due to NMI delivery latency.
>
> When cpuc->lbr_pebs_users > 0 and a non-NMI BPF program (such as an
> fentry/fexit trampoline via __bpf_prog_enter_recur() or a raw_tracepoint,
> where bpf_prog_active is not incremented) calls:
>
>   bpf_get_branch_snapshot()
>     -> intel_pmu_snapshot_branch_stack()
>       -> __intel_pmu_snapshot_branch_stack()
>
> an arriving PMI can still invoke bpf_overflow_handler() ->
> bpf_get_branch_snapshot() -> __intel_pmu_snapshot_branch_stack() on the
> same CPU, overwriting this_cpu_ptr(&lbr_snapshot) while the outer call is
> between its save and restore steps. Even if the PMI does not call
> bpf_get_branch_snapshot(), intel_pmu_handle_irq() updates cpuc->lbr_stack
> and cpuc->lbr_entries while the interrupted snapshot is saving or
> restoring them.
>
> Since cpuc->lbr_stack only holds live sample data awaiting output during a
> PMI, should the save and restore of cpuc->lbr_stack be restricted to NMI
> context or otherwise protected against NMI reentrancy?

This can't happen in reality.  The whole PMU including LBR has been
disabled before calling __intel_pmu_snapshot_branch_stack(). Since PMU
hardware has been disabled, then no PMI can be triggered at the period of
__intel_pmu_snapshot_branch_stack() running. So there is no reentry for the
__intel_pmu_snapshot_branch_stack()  helper, and cpuc->lbr_entries won't be
corrupted.


>
>> +
>>  static noinline int
>>  __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
>>  				  unsigned int cnt, unsigned long flags)
>>  {
>>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>> +	struct lbr_snapshot *snapshot = this_cpu_ptr(&lbr_snapshot);
>> +
>> +	/*
>> +	 * This may be called from bpf_event_handler() in a PMI context. If
>> +	 * a PEBS LBR user is active, the PEBS sample record has already updated
>> +	 * cpuc->lbr_stack. Reading the LBR registers here would overwrite that
>> +	 * cached PEBS LBR stack, so save it first and restore it afterward. This
>> +	 * keeps the cached LBR data available for later output to userspace in
>> +	 * the normal PEBS record path.
>> +	 */
>> +	if (cpuc->lbr_pebs_users) {
>> +		memcpy(&snapshot->stack, &cpuc->lbr_stack,
>> +		       sizeof(struct perf_branch_stack));
>> +		memcpy(snapshot->entries, cpuc->lbr_entries,
>> +		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
>> +		memcpy(snapshot->counters, cpuc->lbr_counters,
>> +		       sizeof(u64) * cpuc->lbr_stack.nr);
>> +	}
> [Severity: Critical]
> What happens in __intel_pmu_snapshot_branch_stack() when cpuc->lbr_users
> is non-zero, but cpuc->lbr_pebs_users is 0?
>
> In arch/x86/events/intel/lbr.c:intel_pmu_lbr_add(), cpuc->lbr_pebs_users
> is only incremented when x86_pmu.intel_cap.pebs_baseline is set and
> event->attr.precise_ip > 0:
>
> 	if (x86_pmu.intel_cap.pebs_baseline && event->attr.precise_ip > 0)
> 		cpuc->lbr_pebs_users++;
>
> When non-PEBS hardware LBR events (precise_ip == 0), legacy PEBS LBR
> events (pre-Ice Lake where pebs_baseline == 0), or Architectural PEBS LBR
> events (arch/x86/events/intel/ds.c:setup_arch_pebs_sample_data(), where
> pebs_baseline == 0) are active, cpuc->lbr_users > 0 while
> cpuc->lbr_pebs_users == 0.
>
> In those paths, handle_pmi_common(), setup_pebs_fixed_sample_data(), and
> setup_arch_pebs_sample_data() call intel_pmu_lbr_save_brstack() before
> perf_event_overflow() -> __perf_event_overflow() ->
> bpf_overflow_handler():
>
> 	if (has_branch_stack(event))
> 		intel_pmu_lbr_save_brstack(&data, cpuc, event);
> 	...
> 	perf_event_overflow(event, &data, regs);
>
> Inside include/linux/perf_event.h:perf_sample_save_brstack(), brs
> (&cpuc->lbr_stack) is clamped in place and used to precompute
> data->dyn_size:
>
> 	brs->nr = min_t(u16, event->attr.sample_max_stack, brs->nr);
> 	size += brs->nr * sizeof(struct perf_branch_entry);
> 	if (brs_cntr)
> 		size += brs->nr * sizeof(u64);
> 	data->br_stack = brs;
> 	data->br_stack_cntr = brs_cntr;
> 	data->dyn_size += size;
> 	data->sample_flags |= PERF_SAMPLE_BRANCH_STACK;
>
> If bpf_overflow_handler() then calls bpf_get_branch_snapshot() ->
> __intel_pmu_snapshot_branch_stack() with cpuc->lbr_pebs_users == 0, the
> save and restore of cpuc->lbr_stack, cpuc->lbr_entries, and
> cpuc->lbr_counters are skipped, and intel_pmu_lbr_read() overwrites them
> in place:
>
> 1. In kernel/events/core.c:perf_output_sample(), data->br_stack still
>    points to &cpuc->lbr_stack. Because intel_pmu_lbr_read() resets
>    cpuc->lbr_stack.nr up to x86_pmu.lbr_nr, undoing the sample_max_stack
>    clamp after data->dyn_size was already computed, perf_output_sample()
>    can copy more perf_branch_entry (and counter) elements than
>    perf_output_begin() reserved in the ring buffer.
>
> 2. The frozen LBR entries captured at PMI entry (or from the Arch PEBS
>    record) in cpuc->lbr_entries are overwritten by unfrozen post-ack LBR
>    registers containing PMI handler branches.
>
> 3. For branch counter groups, intel_pmu_lbr_counters_reorder() has already
>    extracted counter bits from cpuc->lbr_entries[i].reserved into
>    cpuc->lbr_counters[i] and cleared .reserved; calling
>    intel_pmu_lbr_read() -> intel_pmu_store_lbr() writes raw counter bits
>    back into cpuc->lbr_entries[i].reserved and desynchronizes
>    cpuc->lbr_counters[].
>
> Should the save and restore in __intel_pmu_snapshot_branch_stack() be
> gated on cpuc->lbr_users instead of cpuc->lbr_pebs_users (and save/restore
> based on the pre-clamp entry count or full array when needed)?

This is a real issue. For arch-PEBS, the lbr_pebs_users is missed to
account. Would add a new patch to address this issue.


>
>>  
>>  	intel_pmu_lbr_read();
>>  	cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
>> -
>>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
> [Severity: Medium]
> On platforms with Architectural LBR and branch counter logging, does
> copying cpuc->lbr_entries directly after intel_pmu_lbr_read() leak raw
> branch counter bits in entries[i].reserved to BPF programs?
>
> When intel_pmu_snapshot_arch_branch_stack() calls
> __intel_pmu_snapshot_branch_stack(), intel_pmu_lbr_read() invokes
> intel_pmu_arch_lbr_read() or intel_pmu_arch_lbr_read_xsave(), which calls
> arch/x86/events/intel/lbr.c:intel_pmu_store_lbr():
>
> 	/*
> 	 * Leverage the reserved field of
> 	 * cpuc->lbr_entries[i] to temporarily store the
> 	 * branch counters information. The later code will
> 	 * decide what content can be disclosed to the perf
> 	 * tool. Pleae see intel_pmu_lbr_counters_reorder().
> 	 */
> 	.reserved = (info >> LBR_INFO_BR_CNTR_OFFSET) &
> 		    LBR_INFO_BR_CNTR_FULL_MASK,
>
> Normally intel_pmu_lbr_counters_reorder() filters those bits and clears
> cpuc->lbr_entries[i].reserved to 0 before the entries are exposed. Since
> __intel_pmu_snapshot_branch_stack() copies cpuc->lbr_entries directly into
> entries without calling intel_pmu_lbr_counters_reorder(), won't the raw
> hardware counter bits remain in the UAPI perf_branch_entry.reserved field
> returned by bpf_get_branch_snapshot()?

Currently the intel_pmu_lbr_filter() has already filtered the LBR entries
which contain the sensitive information, like kernel address if only user
space sampling is required. After the filtering, there should be no any
sensitive data including the LBR counters. I suppose it can be exposed to
user space.



>
>> +
>> +	if (cpuc->lbr_pebs_users) {
>> +		memcpy(&cpuc->lbr_stack, &snapshot->stack,
>> +		       sizeof(struct perf_branch_stack));
>> +		memcpy(cpuc->lbr_entries, snapshot->entries,
>> +		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
>> +		memcpy(cpuc->lbr_counters, snapshot->counters,
>> +		       sizeof(u64) * cpuc->lbr_stack.nr);
>> +	}
>> +
>>  	intel_pmu_enable_all(0);
> [Severity: High]
> Is it safe for __intel_pmu_snapshot_branch_stack() to unconditionally call
> intel_pmu_enable_all(0) when invoked from bpf_overflow_handler() in a PMI?
>
> In arch/x86/events/intel/core.c:intel_pmu_handle_irq(), the PMI handler
> saves cpuc->enabled, sets cpuc->enabled = 0, and disables the PMU in
> hardware:
>
> 	pmu_enabled = cpuc->enabled;
> 	...
> 	cpuc->enabled = 0;
> 	__intel_pmu_disable_all(true);
>
> The PMI handler expects the PMU, PEBS, BTS, and LBRs to remain disabled in
> hardware while handle_pmi_common() and __intel_pmu_pebs_last_event() drain
> PEBS records and reprogram counter periods, only restoring the hardware
> state at the done label if pmu_enabled was set:
>
> 	/* Only restore PMU state when it's active. See x86_pmu_disable(). */
> 	cpuc->enabled = pmu_enabled;
> 	if (pmu_enabled)
> 		__intel_pmu_enable_all(0, true);
>
> When a BPF program runs via perf_event_overflow() ->
> bpf_overflow_handler() during a PMI and calls bpf_get_branch_snapshot() ->
> __intel_pmu_snapshot_branch_stack(), calling intel_pmu_enable_all(0)
> invokes x86_pmu_pebs_enable_all() and __intel_pmu_enable_all(0, false)
> while cpuc->enabled is 0. This re-enables MSR_IA32_PEBS_ENABLE, LBRs, BTS,
> and MSR_CORE_PERF_GLOBAL_CTRL before __intel_pmu_pebs_last_event() and
> handle_pmi_common() finish reprogramming counter periods, and leaves the
> hardware PMU enabled after the PMI exits if the PMI interrupted an
> x86_pmu_disable() section where pmu_enabled was 0.
>
> Should __intel_pmu_snapshot_branch_stack() avoid calling
> intel_pmu_enable_all(0) when cpuc->enabled is 0?

Yeah, that's a real issue, would fix it in next version.



>
>>  	local_irq_restore(flags);
>>  	return cnt;
>> @@ -4022,7 +4056,12 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
>>  
>>  	loops = 0;
>>  again:
>> -	intel_pmu_lbr_read();
>> +	/*
>> +	 * No need to read the LBR stack when all LBR users are PEBS events,
>> +	 * since the PEBS handler updates it.
>> +	 */
>> +	if (cpuc->lbr_users != cpuc->lbr_pebs_users)
>> +		intel_pmu_lbr_read();
>>  	intel_pmu_ack_status(status);
> [Severity: Medium]
> When cpuc->lbr_users == cpuc->lbr_pebs_users and a BPF program calls
> bpf_get_branch_snapshot() from bpf_overflow_handler() during a PMI, haven't
> the hardware LBR registers already been unfrozen by
> intel_pmu_ack_status(status)?
>
> In arch/x86/events/intel/core.c:intel_pmu_handle_irq(),
> __intel_pmu_disable_all(true) clears MSR_CORE_PERF_GLOBAL_CTRL but does
> not clear LBR enable bits in MSR_IA32_DEBUGCTLMSR or MSR_ARCH_LBR_CTL,
> relying on GLOBAL_STATUS_LBRS_FROZEN to keep the LBR stack frozen.
>
> As noted in arch/x86/events/intel/lbr.c:__intel_pmu_lbr_enable():
>
> 	/*
> 	 * No need to unfreeze manually, as v4 can do that as part
> 	 * of the GLOBAL_STATUS ack.
> 	 */
> 	if (pmi && x86_pmu.version >= 4)
> 		return;
>
> On PMU v4+ platforms (which include all pebs_baseline platforms), calling
> intel_pmu_ack_status(status) clears GLOBAL_STATUS_LBRS_FROZEN and
> immediately unfreezes the hardware LBRs. By the time handle_pmi_common() ->
> x86_pmu_drain_pebs() -> __intel_pmu_pebs_last_event() sets up the PEBS
> sample and invokes bpf_overflow_handler() ->
> __intel_pmu_snapshot_branch_stack(), the unfrozen hardware LBR MSRs have
> been recording branches inside the PMI handler itself (and
> __intel_pmu_snapshot_branch_stack() then also calls intel_pmu_enable_all(0)
> before the PMI finishes).
>
> Can __intel_pmu_snapshot_branch_stack() still obtain an unpolluted hardware
> LBR snapshot in this PMI path after intel_pmu_ack_status(status) has
> unfrozen the LBRs?

Ah, it's indeed an issue. Would fix it in next version.

Thanks.


>
> [ ... ]
>

Re: [Patch v4 5/5] pef/x86/intel: Always snapshot live hardware LBR state for BPF path

[Mi, Dapeng]

Sashiko found several new long-standing LBR issues (not caused by the patch
series), https://lore.kernel.org/all/sashiko-outbox-179823@kernel.org/

Would post new patch series to fix the newly found issues. Thanks.

On 10/21/2026 1:07 PM, Dapeng Mi wrote:
> __intel_pmu_snapshot_branch_stack() is meant to snapshot the current
> runtime LBR state from hardware for the BPF path. However, stale cached
> PEBS LBR data can be snapshotted instead when a PEBS LBR user is active.
>
> Fix this by always snapshotting the live hardware LBR state, regardless
> of whether a PEBS LBR user is active. Reading the runtime LBR registers
> can overwrite the cached PEBS LBR stack, so save and restore the cached
> PEBS LBR state around the snapshot. This preserves the PEBS LBR data for
> later output to userspace in the normal PEBS record path.
>
> Fixes: c22ac2a3d4bd ("perf: Enable branch record for software events")
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---
>  arch/x86/events/intel/core.c | 43 ++++++++++++++++++++++++++++++++++--
>  arch/x86/events/intel/lbr.c  |  9 --------
>  2 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 5dfb9e9156e8..07d1a8a7845b 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -2815,16 +2815,50 @@ static void intel_pmu_enable_all(int added)
>  	__intel_pmu_enable_all(added, false);
>  }
>  
> +struct lbr_snapshot {
> +	struct perf_branch_stack stack;
> +	struct perf_branch_entry entries[MAX_LBR_ENTRIES];
> +	u64 counters[MAX_LBR_ENTRIES];
> +};
> +static DEFINE_PER_CPU(struct lbr_snapshot, lbr_snapshot);
> +
>  static noinline int
>  __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
>  				  unsigned int cnt, unsigned long flags)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> +	struct lbr_snapshot *snapshot = this_cpu_ptr(&lbr_snapshot);
> +
> +	/*
> +	 * This may be called from bpf_event_handler() in a PMI context. If
> +	 * a PEBS LBR user is active, the PEBS sample record has already updated
> +	 * cpuc->lbr_stack. Reading the LBR registers here would overwrite that
> +	 * cached PEBS LBR stack, so save it first and restore it afterward. This
> +	 * keeps the cached LBR data available for later output to userspace in
> +	 * the normal PEBS record path.
> +	 */
> +	if (cpuc->lbr_pebs_users) {
> +		memcpy(&snapshot->stack, &cpuc->lbr_stack,
> +		       sizeof(struct perf_branch_stack));
> +		memcpy(snapshot->entries, cpuc->lbr_entries,
> +		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
> +		memcpy(snapshot->counters, cpuc->lbr_counters,
> +		       sizeof(u64) * cpuc->lbr_stack.nr);
> +	}
>  
>  	intel_pmu_lbr_read();
>  	cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
> -
>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
> +
> +	if (cpuc->lbr_pebs_users) {
> +		memcpy(&cpuc->lbr_stack, &snapshot->stack,
> +		       sizeof(struct perf_branch_stack));
> +		memcpy(cpuc->lbr_entries, snapshot->entries,
> +		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
> +		memcpy(cpuc->lbr_counters, snapshot->counters,
> +		       sizeof(u64) * cpuc->lbr_stack.nr);
> +	}
> +
>  	intel_pmu_enable_all(0);
>  	local_irq_restore(flags);
>  	return cnt;
> @@ -4022,7 +4056,12 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
>  
>  	loops = 0;
>  again:
> -	intel_pmu_lbr_read();
> +	/*
> +	 * No need to read the LBR stack when all LBR users are PEBS events,
> +	 * since the PEBS handler updates it.
> +	 */
> +	if (cpuc->lbr_users != cpuc->lbr_pebs_users)
> +		intel_pmu_lbr_read();
>  	intel_pmu_ack_status(status);
>  	if (++loops > 100) {
>  		static bool warned;
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index ac1759a71288..07b32255e48d 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1030,15 +1030,6 @@ void intel_pmu_lbr_read(void)
>  		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 == cpuc->lbr_pebs_users)
> -		return;
> -
>  	x86_pmu.lbr_read(cpuc);
>  
>  	intel_pmu_lbr_filter(cpuc);