Re: [Patch v9 11/24] perf/x86: Enable XMM register sampling for non-PEBS events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] reserve_ext_regs_buffers() fails to propagate memory allocation errors and improperly uses WARN_ON_ONCE, leading to a silent feature failure and possible kernel panics under memory pressure.
--

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

perf/x86: Enable XMM register sampling for non-PEBS events

This commit extends XMM register sampling to non-PEBS events using the xsaves
instruction. It allocates a per-CPU ext_regs_buf to store SIMD and other
registers. For PEBS events, data is retrieved directly from PEBS records.

> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index d83ea02e24577..3e52610510cdc 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
[ ... ]
> @@ -410,6 +410,56 @@ set_ext_hw_attr(struct hw_perf_event *hwc, struct perf_event *event)
> +static void reserve_ext_regs_buffers(void)
> +{
> +	bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED);
> +	unsigned int size;
> +	int cpu;
> +
> +	if (!x86_pmu.ext_regs_mask)
> +		return;
> +
> +	/* +64 bytes for the 64 bytes alignment request of xsave area. */
> +	size = xstate_calculate_size(x86_pmu.ext_regs_mask, compacted) + 64;
> +
> +	for_each_possible_cpu(cpu) {
> +		per_cpu(ext_regs_buf, cpu) = kzalloc_node(size, GFP_KERNEL,
> +							  cpu_to_node(cpu));
> +		if (WARN_ON_ONCE(!per_cpu(ext_regs_buf, cpu)))
> +			goto err;
> +	}
> +
> +	return;
> +
> +err:
> +	release_ext_regs_buffers();
> +}
[ ... ]
>  int x86_reserve_hardware(void)
>  {
>  	int err = 0;
>  
>  	if (!atomic_inc_not_zero(&pmc_refcount)) {
>  		mutex_lock(&pmc_reserve_mutex);
>  		if (atomic_read(&pmc_refcount) == 0) {
>  			if (!reserve_pmc_hardware()) {
>  				err = -EBUSY;
>  			} else {
>  				reserve_ds_buffers();
>  				reserve_lbr_buffers();
> +				reserve_ext_regs_buffers();
>  			}
>  		}
>  		if (!err)

[Severity: High]
Does this code swallow the memory allocation failure?

Since reserve_ext_regs_buffers() returns void, x86_reserve_hardware() 
appears unaware if kzalloc_node() fails under memory pressure. The hardware 
reservation will succeed anyway, and the perf event creation is permitted
even though the backing buffers do not exist.

[ ... ]
> +static void x86_pmu_sample_xregs(struct perf_event *event,
> +				 struct perf_sample_data *data,
> +				 u64 ignore_mask)
> +{
> +	struct xregs_state *xsave = get_ext_regs_buf(smp_processor_id());
> +	u64 sample_type = event->attr.sample_type;
> +	struct x86_perf_regs *perf_regs;
> +	u64 intr_mask = 0;
> +	u64 mask = 0;
> +
> +	if (WARN_ON_ONCE(!xsave) || !in_nmi())
> +		return;

[Severity: High]
Can this path be reached with a NULL xsave buffer during a PMI if the earlier
allocation failed?

If an unprivileged user creates an event requesting extended registers during
memory pressure, and the allocation in reserve_ext_regs_buffers() fails,
the event is successfully created. When the PMI hits, this WARN_ON_ONCE in
NMI context will trigger.

This could panic the system if panic_on_warn is set, or otherwise silently
drop the requested register data. Should reserve_ext_regs_buffers() return
an error code so x86_reserve_hardware() can fail the event creation instead?

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