Re: [PATCH v2 7/9] perf/cxl: Validate the hardware-reported counter width

On Wed, 29 Jul 2026 07:55:53 -0700
Dave Jiang <[email protected]> wrote:

> cxl_pmu_parse_caps() takes the Counter Width straight from the CPMU
> Capability register with no bounds check. The Counter Data register is
> 64 bits, so a device reporting width 0 (or > 64) is invalid, and the
> read path's GENMASK_ULL(counter_width - 1, 0) would shift out of range
> (undefined behavior, e.g. a UBSAN splat) on a value of 0.
> 
> Reject a counter width outside 1..64 at probe.
> 
> Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")

I don't mind hardening this but there has been a lot of push back on
'bugs' that are dependent on broken hardware (part of the AI discussions
on the ksummit list).  

Anyhow it is easy protection so fair enough - I'm just not sure it is
worth the pain of chasing down more complex hardware did the wrong thing
'bugs'

Reviewed-by: Jonathan Cameron <[email protected]>



> Reported-by: [email protected]
> Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <[email protected]>
> ---
>  drivers/perf/cxl_pmu.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 52e78a6e0960..2e817a52ff1e 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -139,6 +139,15 @@ static int cxl_pmu_parse_caps(struct device *dev, struct cxl_pmu_info *info)
>  
>  	info->num_counters = FIELD_GET(CXL_PMU_CAP_NUM_COUNTERS_MSK, val) + 1;
>  	info->counter_width = FIELD_GET(CXL_PMU_CAP_COUNTER_WIDTH_MSK, val);
> +	/*
> +	 * The Counter Data register is 64 bits wide, so a Counter Width of 0 or
> +	 * >64 is invalid. Reject it rather than let GENMASK_ULL(width - 1, 0) in
> +	 * the read path shift out of range.
> +	 */
> +	if (info->counter_width == 0 || info->counter_width > 64) {
> +		dev_err(dev, "Invalid counter width %d\n", info->counter_width);
> +		return -ENODEV;
> +	}
>  	info->num_event_capabilities = FIELD_GET(CXL_PMU_CAP_NUM_EVN_CAP_REG_SUP_MSK, val) + 1;
>  
>  	info->filter_hdm = FIELD_GET(CXL_PMU_CAP_FILTERS_SUP_MSK, val) & CXL_PMU_FILTER_HDM;