Re: [PATCH 7/7] perf/x86/intel/uncore: Implement lazy setup for MSR/MMIO PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Local parameter modification in uncore_cpu_mmio_init fails to clear global uncore arrays on error, leading to a guaranteed kernel crash (Use-After-Free/NULL dereference) on partial initialization failure.
--

commit 27fb6ccf46a9e646f34ac7d3e4098f98a775ed6f
Author: Zide Chen <zide.chen@intel.com>

perf/x86/intel/uncore: Implement lazy setup for MSR/MMIO PMU

This commit updates MSR and MMIO uncore PMUs to use a lazy registration
model. The PMUs are registered when the first box is successfully
initialized and unregistered when the last box exits. Box allocation and
freeing remain at module init/exit time.

> diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
> index b9b901b789d15..c6fe7864a4977 100644
> --- a/arch/x86/events/intel/uncore.c
> +++ b/arch/x86/events/intel/uncore.c

[ ... ]

> -static int __init uncore_cpu_init(void)
> -{
> -	int ret;
> -
> -	ret = uncore_types_init(uncore_msr_uncores);
> -	if (ret)
> -		goto err;
> -
> -	ret = uncore_msr_pmus_register();
> -	if (ret)
> -		goto err;
> -	return 0;
> -err:
> -	uncore_types_exit(uncore_msr_uncores);
> -	uncore_msr_uncores = empty_uncore;
> -	return ret;
> -}

[ ... ]

> +static int __init uncore_cpu_mmio_init(struct intel_uncore_type **types)
>  {
>  	int ret;
>  
>  	ret = uncore_types_init(types);
> +	if (!ret)
> +		return 0;
>  
> +	uncore_types_exit(types);
> +	types = empty_uncore;

Does this error path effectively clear the global uncore arrays?

Since types is passed by value, assigning types = empty_uncore only updates
the local parameter. The global variables like uncore_msr_uncores or
uncore_mmio_uncores are left pointing to structures that were just cleaned
up by uncore_types_exit().

If intel_uncore_init() experiences a partial failure, it can still proceed
to register CPU hotplug callbacks. This synchronously triggers
uncore_event_cpu_online() which iterates over the global arrays:

intel_uncore_init()
  uncore_cpu_mmio_init()
  cpuhp_setup_state()
    uncore_event_cpu_online()
      allocate_boxes()

Could this access freed structures and result in a use-after-free or NULL
pointer dereference since the global array was not cleared?

>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512233048.9577-1-zide.chen@intel.com?part=7