Re: [PATCH 2/5] cpufreq: scmi-cpufreq: Log number of perf domains

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Use-After-Free (UAF) vulnerability due to global state overwrite in multi-SCMI configurations.
- [Low] Misleading and factually incorrect log message regarding initialized performance domains.
--

commit 9067d109b9e937b5bfb6e9d3b3c94d2e9b71d9ac
Author: Alex Tran <alex.tran@oss.qualcomm.com>

cpufreq: scmi-cpufreq: Log number of perf domains

This commit adds a log message to the SCMI cpufreq driver probe function
to display the number of performance domains discovered from firmware.
This is intended to align with the existing SCMI client driver logging
pattern.

> diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
> index 4edb4f7a8aa97..d5c33eea24773 100644
> --- a/drivers/cpufreq/scmi-cpufreq.c
> +++ b/drivers/cpufreq/scmi-cpufreq.c
> @@ -468,9 +468,12 @@ static int scmi_cpufreq_probe(struct scmi_device *sdev)
>  	if (ret) {
>  		dev_err(dev, "%s: registering cpufreq failed, err: %d\n",
>  			__func__, ret);
> +		return ret;
>  	}
>  
> -	return ret;
> +	dev_info(dev, "Initialized %d performance domains\n",
> +		 perf_ops->num_domains_get(ph));

Is this log message entirely accurate?

The num_domains_get() function returns the total number of performance
domains exposed by the SCMI firmware, which includes domains for other
devices like GPUs or DSPs.

Since the scmi-cpufreq driver only initializes domains explicitly associated
with CPUs, could this log message be misleading during debugging by claiming
all SCMI performance domains were initialized here?

This isn't a bug introduced by this commit, but does the use of the global
perf_ops and ph variables here expose a use-after-free on systems with
multiple SCMI agents?

When a second device probes, it overwrites the global perf_ops and ph handles
earlier in the function:

    perf_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PERF, &ph);

The second probe then calls cpufreq_register_driver(), which fails with
-EEXIST because a cpufreq driver is already registered.

The probe failure triggers devres cleanup for the second device, which calls
scmi_protocol_release() and frees the protocol instance.

Since the global ph pointer now points to freed memory, will subsequent
frequency transitions for the first device's CPUs dereference a dangling
pointer?

> +	return 0;
>  }
>  
>  static void scmi_cpufreq_remove(struct scmi_device *sdev)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-scmi-client-probe-log-v1-0-00b47b1be009@oss.qualcomm.com?part=2