[RFC PATCH] perf/amd/ibs: Report physical address for IBS fetch samples

[RFC PATCH] perf/amd/ibs: Report physical address for IBS fetch samples

[Huang Shijie]

The IBS Fetch sampling does not report the physical address of the
fetched instruction even when PERF_SAMPLE_PHYS_ADDR is requested,
while IBS Op sampling does.

This patch reports physical address for IBS fetch samples which can be
used for profiling the running program.

Signed-off-by: Huang Shijie <huangsj@hygon.cn>
---
 arch/x86/events/amd/ibs.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 3531f9c23b8c..e5376ad5b2ec 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -1318,11 +1318,31 @@ static void perf_ibs_parse_ld_st_data(__u64 sample_type,
 	}
 }
 
+static void perf_ibs_parse_fetch_data(__u64 sample_type,
+				      struct perf_ibs_data *ibs_data,
+				      struct perf_sample_data *data)
+{
+	union ibs_fetch_ctl fetch_ctl;
+
+	if (!(sample_type & PERF_SAMPLE_PHYS_ADDR))
+		return;
+
+	fetch_ctl.val = ibs_data->regs[ibs_fetch_msr_idx(MSR_AMD64_IBSFETCHCTL)];
+	if (!fetch_ctl.phy_addr_valid)
+		return;
+
+	data->phys_addr = ibs_data->regs[ibs_fetch_msr_idx(MSR_AMD64_IBSFETCHPHYSAD)];
+	data->sample_flags |= PERF_SAMPLE_PHYS_ADDR;
+}
+
 static bool perf_ibs_is_mem_sample_type(struct perf_ibs *perf_ibs,
 					struct perf_event *event)
 {
 	u64 sample_type = event->attr.sample_type;
 
+	if (perf_ibs == &perf_ibs_fetch)
+		return sample_type & PERF_SAMPLE_PHYS_ADDR;
+
 	return perf_ibs == &perf_ibs_op &&
 	       sample_type & (PERF_SAMPLE_DATA_SRC |
 			      PERF_SAMPLE_WEIGHT_TYPE |
@@ -1555,6 +1575,8 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 
 	if (perf_ibs == &perf_ibs_op)
 		perf_ibs_parse_ld_st_data(event->attr.sample_type, &ibs_data, &data);
+	else
+		perf_ibs_parse_fetch_data(event->attr.sample_type, &ibs_data, &data);
 
 	/*
 	 * rip recorded by IbsOpRip will not be consistent with rsp and rbp
-- 
2.53.0

Re: [RFC PATCH] perf/amd/ibs: Report physical address for IBS fetch samples

[sashiko-bot]

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

Pre-existing issues:
- [High] Kernel stack buffer overflow when handling AMD IBS Op samples with both IBS_CAPS_BRNTRGT and IBS_CAPS_OPDATA4 capabilities.
--

--- Patch [1]: [RFC PATCH] perf/amd/ibs: Report physical address for IBS fetch samples ---
commit 96b9a5e865d821f72dd58d33f3f265a7de5821e5
Author: Huang Shijie <huangsj@hygon.cn>

perf/amd/ibs: Report physical address for IBS fetch samples

This commit updates AMD IBS fetch sampling to report the physical address of
fetched instructions when PERF_SAMPLE_PHYS_ADDR is requested. It implements
this by parsing the fetch data similarly to how operations data is handled.

> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index 3531f9c23b8c9..e5376ad5b2ecd 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c

[ ... ]

> @@ -1555,6 +1575,8 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
>  
>  	if (perf_ibs == &perf_ibs_op)
>  		perf_ibs_parse_ld_st_data(event->attr.sample_type, &ibs_data, &data);
> +	else
> +		perf_ibs_parse_fetch_data(event->attr.sample_type, &ibs_data, &data);

[Severity: High]
This isn't a bug introduced by this patch, but while looking at how
ibs_data is handled in perf_ibs_handle_irq(), I noticed a potential kernel
stack buffer overflow.

Does the code overflow the ibs_data.regs[] array when a CPU supports both
IBS_CAPS_BRNTRGT and IBS_CAPS_OPDATA4 capabilities?

MSR_AMD64_IBS_REG_COUNT_MAX is defined as 8 in asm/msr-index.h. If both
capabilities are present, it appears 7 MSRs are read into indices 0 to 6,
MSR_AMD64_IBSBRTARGET is read into index 7, and then MSR_AMD64_IBSOPDATA4
would be written to index 8, writing past the end of the array.

Could this also cause uninitialized kernel stack memory to be exposed to
userspace via the perf ring buffer since ibs_data.size would evaluate to 72
bytes, making raw.frag.size 76 bytes, while the valid struct space is only
68 bytes?

>  
>  	/*
>  	 * rip recorded by IbsOpRip will not be consistent with rsp and rbp

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