Re: [PATCH] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds stack write to ibs_data.regs in perf_ibs_handle_irq when IBS_CAPS_OPDATA4 is present.
--

commit 5394e86487207a321dbcef29ca4cd958bfbe243c
Author: Huang Shijie <huangsj@hygon.cn>

perf/amd/ibs: add phy_addr_only software filter for ibs_op

This commit introduces an ibs_op/phy_addr_only filter to discard samples
without a valid physical address directly in the interrupt handler. This
avoids wasting CPU cycles and reduces noise when tracking a process's memory
footprint using perf record.

> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index 3531f9c23b8c9..754f6f10b1717 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
[ ... ]
> @@ -1479,6 +1490,16 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
>  		}
>  	}
>  
> +	if (perf_ibs_phy_addr_only_event(perf_ibs, event)) {
> +		union ibs_op_data3 op_data3;
> +
> +		op_data3.val = ibs_data.regs[ibs_op_msr_idx(MSR_AMD64_IBSOPDATA3)];
> +		if (!op_data3.dc_phy_addr_valid) {
> +			throttle = perf_event_account_interrupt(event);
> +			goto out;
> +		}
> +	}
> +
>  	if (perf_ibs_fetch_lat_event(perf_ibs, event)) {
>  		union ibs_fetch_ctl fetch_ctl;
>  

[Severity: High]
This isn't a bug introduced by this patch, but is there an out-of-bounds
write to the ibs_data.regs[] array further down in perf_ibs_handle_irq()?

The array is sized to MSR_AMD64_IBS_REG_COUNT_MAX, which is 8:

arch/x86/include/asm/msr-index.h:
#define MSR_AMD64_IBS_REG_COUNT_MAX 8

When reading MSRs, if the hardware supports both IBS_CAPS_BRNTRGT and
IBS_CAPS_OPDATA4, the code reads up to 7 MSRs into the buffer in a loop
and then continues:

arch/x86/events/amd/ibs.c:perf_ibs_handle_irq() {
    ...
		if (perf_ibs == &perf_ibs_op) {
			if (ibs_caps & IBS_CAPS_BRNTRGT) {
				rdmsrq(MSR_AMD64_IBSBRTARGET, *buf++);
				br_target_idx = size;
				size++;
			}
			if (ibs_caps & IBS_CAPS_OPDATA4) {
				rdmsrq(MSR_AMD64_IBSOPDATA4, *buf++);
				size++;
			}
    ...

The rdmsrq() for IBS_CAPS_BRNTRGT writes into regs[7] and increments buf to
regs[8]. The rdmsrq() for IBS_CAPS_OPDATA4 then writes into regs[8], which is
past the end of the array.

Could this corrupt the kernel stack in NMI context when a user requests
PERF_SAMPLE_RAW?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902085251.4046240-1-huangsj@hygon.cn?part=1