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

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

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.




>
>>  
>  완벽
>