[PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Sandipan Das]

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

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

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

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

Re: [PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Peter Zijlstra]

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

You now seem to deviate from what the Intel code does. Please clarify.

Re: [PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Sandipan Das]

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

Yes, it is a deviation. I thought of proposing the following but didn't
proceed because I was not sure about the if condition in the second hunk
which considers things other than cpuc->lbr_users for an early return.

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 0a34d674df59..5dfb9e9156e8 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2822,7 +2822,7 @@ __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
        struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);

        intel_pmu_lbr_read();
-       cnt = min_t(unsigned int, cnt, x86_pmu.lbr_nr);
+       cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);

        memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
        intel_pmu_enable_all(0);
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 0b446d8912e4..6b070490f461 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1026,8 +1026,10 @@ void intel_pmu_lbr_read(void)
         * but this simple approach seems to work for now.
         */
        if (!cpuc->lbr_users || vlbr_exclude_host() ||
-           cpuc->lbr_users == cpuc->lbr_pebs_users)
+           cpuc->lbr_users == cpuc->lbr_pebs_users) {
+               cpuc->lbr_stack.nr = 0;
                return;
+       }

        x86_pmu.lbr_read(cpuc);

Re: [PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Peter Zijlstra]

On Tue, Oct 13, 2026 at 12:14:58PM +0530, Sandipan Das wrote:
> On 12-10-2026 21:00, Peter Zijlstra wrote:
> > On Wed, Oct 07, 2026 at 11:40:02PM +0530, Sandipan Das wrote:
> >> The branch snapshot callback used by bpf_get_branch_snapshot() copies
> >> up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
> >> last LBR read kept fewer branches. The entries past lbr_stack.nr are
> >> invalid, but they end up reaching BPF programs.
> >>
> >> Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
> >> amd_pmu_lbr_read() skips reading the registers, so that a stale count
> >> is never reused.
> >>
> >> Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
> >> Reported-by: Sashiko <sashiko-bot@kernel.org>
> >> Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
> >> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> >> ---
> >>  arch/x86/events/amd/core.c | 2 +-
> >>  arch/x86/events/amd/lbr.c  | 4 +++-
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> >> index 49b6b8fce566..bf631f60757c 100644
> >> --- a/arch/x86/events/amd/core.c
> >> +++ b/arch/x86/events/amd/core.c
> >> @@ -944,7 +944,7 @@ static int amd_pmu_v2_snapshot_branch_stack(struct perf_branch_entry *entries, u
> >>  	cpuc = this_cpu_ptr(&cpu_hw_events);
> >>  
> >>  	amd_pmu_lbr_read();
> >> -	cnt = min(cnt, x86_pmu.lbr_nr);
> >> +	cnt = min(cnt, cpuc->lbr_stack.nr);
> >>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
> >>  
> >>  	amd_pmu_v2_enable_all(0);
> >> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
> >> index 1e1b2f08cb51..a00d3850c4a8 100644
> >> --- a/arch/x86/events/amd/lbr.c
> >> +++ b/arch/x86/events/amd/lbr.c
> >> @@ -165,8 +165,10 @@ void amd_pmu_lbr_read(void)
> >>  	struct branch_entry entry;
> >>  	int out = 0, idx, i;
> >>  
> >> -	if (!cpuc->lbr_users)
> >> +	if (!cpuc->lbr_users) {
> >> +		cpuc->lbr_stack.nr = 0;
> >>  		return;
> >> +	}
> >>  
> >>  	for (i = 0; i < x86_pmu.lbr_nr; i++) {
> >>  		entry.from.full	= amd_pmu_lbr_get_from(i);
> > 
> > You now seem to deviate from what the Intel code does. Please clarify.
> 
> Yes, it is a deviation. I thought of proposing the following but didn't
> proceed because I was not sure about the if condition in the second hunk
> which considers things other than cpuc->lbr_users for an early return.

Dapeng, can you look into this? I would really like to keep behaviour
consistent where possible.

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